-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add/remove view UI #359
Add/remove view UI #359
Conversation
Create view mutations
…e added if table currently has a filter, and modal is updated accordingly. adding a view also uses the value of sample_table_index from config if it is present, otherwise it uses "sample_name" by default.
@@ -3,9 +3,11 @@ import { OverlayTrigger, Tooltip } from 'react-bootstrap'; | |||
import { useProjectPageView } from '../../hooks/stores/useProjectPageView'; | |||
import { ViewSelector } from '../project/view-selector'; | |||
|
|||
type PageView = 'samples' | 'subsamples' | 'config'; | |||
type PageView = 'samples' | 'subsamples' | 'config' | 'help'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we think we are going to remove this?
<div className={pageView === 'help' ? 'border-0 px-1 h-100 text-muted bg-white shadow-sm' : 'px-1 h-100'}> | ||
<ViewButton | ||
view="help" | ||
setPageView={setPageView} | ||
icon="bi bi-question-circle-fill me-2" | ||
text="Help" | ||
isDirty={false} | ||
bold={pageView === 'help' ? ' fw-normal' : ' fw-light'} | ||
color={pageView === 'help' ? ' text-dark' : ' text-muted'} | ||
/> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we think we will still keep this?
useEffect(() => { | ||
if (projectConfigQuery.data?.config) { | ||
const index = extractSampleTableIndex(projectConfigQuery.data.config); | ||
setSampleTableIndex(index); | ||
} | ||
}, [projectConfigQuery.data]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to get away without an effect here.
Just something like:
const sampleTableIndex = projectConfigQuery ? extractSampleTableIndex(projectConfigQuery.data.config) : 'sample_name';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try to avoid useEffect
at all costs
Adding views from filtered sample tables works, but the view selector in projects has a bug as of a couple weeks ago and doesn't actually filter the handsontable. The views themselves are good though. You can create a view from dev_sam with a filtered sample table, then git checkout 83e7fe and choose the view you created to display the correctly filtered rows.