-
-
Notifications
You must be signed in to change notification settings - Fork 2
TT-6812 + corrections #135
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
base: dev-44
Are you sure you want to change the base?
Conversation
| import { getDataGridLocale } from './utils/dataGridLocale'; | ||
| import { useMemo } from 'react'; | ||
| export const HeadHeight = 64; | ||
| export const HeadHeight = 56; |
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 need to only adjust this when on mobile? Or will this work for the desktop/web also?
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 made this change as well as the change in PlanScreen with the assumption that the header can stay the same between mobile and the new desktop view when we implement it. If this is not the case then I can adjust it accordingly.
| .filter((s) => related(s, 'plan') === plan) | ||
| .sort((i, j) => i.attributes?.sequencenum - j.attributes?.sequencenum) | ||
| : []; | ||
| let sectionfiltered = false; |
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.
This seems overly complicated. Why can't we always pass in a SectionD array and a PassageD array??
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 addressed this in a follow up commit
| alt={curUser.attributes?.name || ''} | ||
| src={source} | ||
| sx={avatarSize(small)} | ||
| /> |
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.
does removing this work for all places it's used???
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 addressed this in a follow up commit
| <AppHead switchTo={false} /> | ||
| <PlanProvider> | ||
| <Box id="PlanScreen" sx={{ display: 'flex', paddingTop: '7.5rem' }}> | ||
| <Box id="PlanScreen" sx={{ display: 'flex', paddingTop: '120px' }}> |
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.
does this work for desktop/web also?
|
Nathan, Greg will merge this into the develop branch instead of the dev-44 branch when he gets a chance. All future development will be on the develop branch. Thanks for your work on this!! |
This includes the initial commit for this issue, addressing copilot concerns, and adjusting for new data structure.