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.

Sign up for GitHub

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

Jump to bottom

Prevent Calendar Popper from Overflowing #1577

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Prevent Calendar Popper from Overflowing #1577

merged 2 commits into from
Aug 15, 2024

Conversation

rdonigian
Copy link
Contributor

@rdonigian rdonigian requested a review from CarsonF as a code owner August 2, 2024 16:13
CarsonF
CarsonF reviewed Aug 2, 2024
View reviewed changes
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

😧 can you explain this change? Why isn't it the default?

@rdonigian
Copy link
Contributor Author

😧 can you explain this change? Why isn't it the default?

Renato posted the image of what's going on when a user zooms, which is cutting off the bottom of the calendar from an overflow issue. This ensures the calendar stays on the page as a whole.

@rdonigian
Copy link
Contributor Author

Here is a before and after
Screenshot 2024-08-02 at 1 42 30 PM
Screenshot 2024-08-02 at 1 42 50 PM

@rdonigian
Copy link
Contributor Author

I don't believe I see anything in our config that's preventing that from being the default.

willdch
willdch approved these changes Aug 7, 2024
View reviewed changes
Copy link
Contributor

@willdch willdch left a comment

Choose a reason for hiding this comment

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

I like it, I like it a lot.
Great work @rdonigian

CarsonF
CarsonF approved these changes Aug 8, 2024
View reviewed changes
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

LGTM. Can we just add a comment saying this modifier prioritizes keeping the popup on screen sacrificing keeping the text input visible?

@rdonigian rdonigian force-pushed the 0097-fix-DateField branch from eaea20b to e657249 Compare August 8, 2024 18:34
CarsonF
CarsonF approved these changes Aug 8, 2024
View reviewed changes
Copy link
Member

@CarsonF CarsonF left a comment

Choose a reason for hiding this comment

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

Thanks @rdonigian

@CarsonF CarsonF merged commit f5d9bae into develop Aug 15, 2024
3 checks passed
@CarsonF CarsonF deleted the 0097-fix-DateField branch August 15, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willdch willdch willdch approved these changes

@CarsonF CarsonF CarsonF approved these changes

@andrewmurraydavid andrewmurraydavid Awaiting requested review from andrewmurraydavid

@brentkulwicki brentkulwicki Awaiting requested review from brentkulwicki

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rdonigian @CarsonF @willdch