Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miscfeatures #120

Merged
merged 10 commits into from
Sep 6, 2023
Merged

Miscfeatures #120

merged 10 commits into from
Sep 6, 2023

Conversation

NishiPhalke
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Sep 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screen ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 6, 2023 7:02pm

@@ -41,6 +41,17 @@ const nextConfig = {
trailingSlash: false,
assetPrefix: assetPrefix,
basePath: basePath,

webpack: (config, { isServer }) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genome browser package uses "fs" default file system module of nodejs. Hence I was getting Module not found error. Added below code in next config to resolve this error. Reference (https://stackoverflow.com/questions/64926174/module-not-found-cant-resolve-fs-in-next-js-application)

Copy link
Member

@jpfisher72 jpfisher72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for doing this all. Main things are being consistent with where query and type definitions are. We should probably be on the same page with where type and query definitions are put when the type/query is used once vs repeatedly. We have a mix of putting them in separate types/queries.ts files and keeping them where they're used. Not sure what you think is best.

@@ -42,8 +42,9 @@
"@mui/icons-material": "^5.14.3",
"@mui/lab": "^5.0.0-alpha.137",
"@mui/material": "^5.14.2",
"@types/node": "^20.5.0",
"@types/react": "^18.2.20",
"@mui/styles": "^5.14.5",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What depends on @mui/styles? Genome browser?

https://mui.com/system/styles/basics/
Above link says that this is a legacy solution that doesn't officially support React 18. If possible it'd be good to not use it, but if there's no easy workaround let's keep it for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not ideal to have such a huge constants file sitting in the repository? As in would it be ideal to fetch this file versus define it here? I honestly don't know what is best practice for this case. Good for now.

import React, { RefObject, useEffect, useMemo, useState } from "react"
import { EmptyTrack } from "umms-gb"
import { RequestError } from "umms-gb/dist/components/tracks/trackset/types"
import { ValuedPoint } from "umms-gb/dist/utils/types"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

umms-gb should ideally be setup to be able to import these without referring to a location deep within the package, but okay for right now.

["CTCF", "#00b0d0"],
])

export const BIOSAMPLE_QUERY = gql`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be moved to a queries.ts file?

["CTCF-only", "CTCF-only"],
])

const QUERY = gql`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like other comment, should these live in a dedicated queries.ts file?

import { useRouter } from "next/navigation"
export type QueryResponse = [number, string[], any, [string, string, string, string, string, string][], string[]]

const GENE_AUTOCOMPLETE_QUERY = `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Queries.ts file? Or is it best to define queries where they're used if they're only used once?


const onSearchChange = async (value: string) => {
setOptions([])
const response = await fetch("https://ga.staging.wenglab.org/graphql", {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This url should probably be put in a config file

import { debounce } from "@mui/material/utils"
import { useRouter } from "next/navigation"

const SNP_AUTOCOMPLETE_QUERY = `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like above comment, maybe put in queries file or leave as is if it's not used elsewhere

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying the genomic region searched for is not working for me. I think it's because Autocomplete's options Array does not contain the modified value so it's value never reflects user changes. Should we look into having a second input for the genomic region to search?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try using the "freeSolo" prop on autocomplete as a quick fix. https://mui.com/material-ui/react-autocomplete/#free-solo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, is the list of genes for human and mouse identical?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Completely different

@NishiPhalke NishiPhalke merged commit 319f399 into main Sep 6, 2023
3 checks passed
@NishiPhalke NishiPhalke deleted the miscfeatures branch October 3, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants