-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
fix: Set fixed maxWidth of the cron schedule modal #19485
fix: Set fixed maxWidth of the cron schedule modal #19485
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19485 +/- ##
==========================================
- Coverage 66.54% 66.50% -0.05%
==========================================
Files 1692 1714 +22
Lines 64775 64974 +199
Branches 6661 6717 +56
==========================================
+ Hits 43103 43209 +106
- Misses 19972 20058 +86
- Partials 1700 1707 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Yeah - the min width should be set to what we see in the second half of the "After" video so it doesn't jump when switching to month and year selections |
resolved comment @rusackas |
@@ -71,6 +71,7 @@ export const StyledScheduleTitle = styled.div` | |||
|
|||
export const StyledCronPicker = styled(CronPicker)` | |||
margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; | |||
width: 450px; |
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.
width: 450px; | |
width: ${({ theme }) => theme.gridUnit * 120}px; |
Is 480px too much? This seems like a nice round-ish number of gridUnits (divisible by 3 or 4)
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.
480 px is too much from my memory. Let me check again.
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.
480px seems okay. I will use the above calculation.
/testenv up |
@rusackas Ephemeral environment spinning up at http://34.219.21.66:8080. Credentials are |
@rusackas , the test environment above you created, doesn't have report/alert config on. So we can't see this modal unfortunately there. |
Ephemeral environment shutdown and build artifacts deleted. |
* fix: Set fixed maxWidth of the cron schedule modal * resolve comment * resolve comment
* fix: Set fixed maxWidth of the cron schedule modal * resolve comment * resolve comment
SUMMARY
When we select
year
/month
cases from cron schedule modal, the modal width is changed dynamically.We need to set fixed max width of the modal.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
FCC New Coder Survey 2018 - 1 April 2022 - Watch Video
After:
FCC New Coder Survey 2018 - 1 April 2022 - Watch Video
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION