MDE/PKFE-31 #57
MDE/PKFE-31 #57
Conversation
|
Visual bug
|
| if (!gnomadFile) { | ||
| gnomadErrorStateUpdate('Please select a gnomAD file'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If no files are selected and a button is clicked, it only highlights the first file as missing. Would be better to remove both return checks and after that add if (!gnomadFile || !lovdFile) return;
There was a problem hiding this comment.
In addition, the 'Please select a X file' should be removed since it's not fully readable anyways. Keeping the selection component the same and just changing the border color and text color to red should be enough.
There was a problem hiding this comment.
| if (!clinvarFile) { | ||
| clinvarErrorStateUpdate('Please select a ClinVar file'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If no files are selected and a button is clicked, it only highlights the first file as missing. Would be better to remove both return checks and after that add if (!lovdFile || !clinvarFile) return;
In addition, the 'Please select a X file' should be removed since it's not fully readable anyways. Keeping the selection component the same and just changing the border color and text color to red should be enough.
There was a problem hiding this comment.
| }; | ||
| const applySpliceAiClick = useCallback(async () => { | ||
| if (!applyTo) { | ||
| applyErrorStateUpdate('Please select a file'); |
There was a problem hiding this comment.
No need for the long text inside a file selector, could just leave the component red
There was a problem hiding this comment.
|
|
||
| const applyCaddClick = useCallback(async () => { | ||
| if (!applyTo) { | ||
| applyErrorStateUpdate('Please select a file'); |
There was a problem hiding this comment.
No need for the long text inside a file selector, could just leave the component red
There was a problem hiding this comment.
| }, [fileTree]); | ||
|
|
||
| return ( | ||
| <Box sx={{ display: 'grid', gridTemplateColumns: '50% 50%', p: '1rem' }}> |
There was a problem hiding this comment.
Why not just use a Grid component?
There was a problem hiding this comment.
It's personal
| ? Theme.palette.error.main | ||
| : Theme.palette.text.primary, | ||
| }} | ||
| > |
There was a problem hiding this comment.
Since it's being reused elsewhere in the code, maybe it should've been made as a seperate component with parameters? Passing 'error' parameter would be enough since it could take 'blocked' from context.
There was a problem hiding this comment.
| /> | ||
| } | ||
| label={ | ||
| <StyledGroupParamsTypography sx={{ color: blocked ? Theme.palette.action.disabled : 'initial' }}> |
There was a problem hiding this comment.
'initial' color needs to be changed to Theme.palette.text.primary
| } | ||
| label={ | ||
| <StyledGroupParamsTypography sx={{ color: blocked ? Theme.palette.action.disabled : 'initial' }}> | ||
| Override |
There was a problem hiding this comment.
It would also be nice to rename 'Override' to 'Override file' for clarity
There was a problem hiding this comment.
| })} | ||
| </StyledGroupParamsSelect> | ||
| </FormControl> | ||
| <FormControlLabel |
There was a problem hiding this comment.
It would be nice to have this component hidden when the value selected is "New file..."
There was a problem hiding this comment.
| </StyledGroupParamsTypography> | ||
| } | ||
| labelPlacement='start' | ||
| /> |
There was a problem hiding this comment.
Override component changes like in applyGroupParams also apply here.
There was a problem hiding this comment.
| </StyledGroupParamsTypography> | ||
| } | ||
| labelPlacement='start' | ||
| /> |
There was a problem hiding this comment.
Override component changes like in applyGroupParams also apply here.
There was a problem hiding this comment.
Fixed with MDE/PKFE-31 visual bug-fix |
|
Created new story |
Initial 2024-09-08
Requested changes 2024-09-09