-
Notifications
You must be signed in to change notification settings - Fork 625
fix(SelectPanel): update input fontSize to 16 on small viewports on iOS #6139
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
Conversation
🦋 Changeset detectedLatest commit: 5dc4c8f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
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.
Pull Request Overview
This patch ensures input elements in SelectPanel
use a 16px font size on narrow viewports to prevent iOS from zooming in when text is smaller than 16px.
- Introduces a
fullScreenOnNarrow
prop passed throughSelectPanel
to the filtered list. - Extends
FilteredActionListWithModernActionList
to accept and act onfullScreenOnNarrow
. - Adds a CSS rule to bump the input’s font size on narrow screens.
- Updates the changeset for a patch release.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/SelectPanel/SelectPanel.tsx | Pass fullScreenOnNarrow flag into the list-rendering logic |
packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx | Add fullScreenOnNarrow prop and conditionally apply the CSS class |
packages/react/src/FilteredActionList/FilteredActionList.module.css | Define .FullScreenTextInput with a media query for narrow viewports |
.changeset/mean-parrots-heal.md | Record the patch release and summarise the fix |
Comments suppressed due to low confidence (1)
packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx:46
- The new
fullScreenOnNarrow
prop introduces visual behavior changes but is not covered by existing tests. Please add unit or Visual Regression tests to confirm the font-size adjustment on narrow viewports.
fullScreenOnNarrow?: boolean
@@ -224,6 +226,7 @@ export function FilteredActionList({ | |||
aria-describedby={inputDescriptionTextId} | |||
loaderPosition={'leading'} | |||
loading={loading && !loadingType.appearsInBody} | |||
className={fullScreenOnNarrow ? classes.FullScreenTextInput : ''} |
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 direct className
assignment will override any className
provided in textInputProps
, potentially dropping existing styling. Consider merging them, for example using clsx(textInputProps.className, fullScreenOnNarrow && classes.FullScreenTextInput)
.
Copilot uses AI. Check for mistakes.
… fix/selectpanel-zoomin
@@ -4,3 +4,12 @@ | |||
overflow: auto; | |||
flex-grow: 1; | |||
} | |||
|
|||
.FullScreenTextInput { |
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.
Just curious, do you need a classname at all since you're gating the change inside a media query?
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'm using it on js side to only apply it when the SelectPanel is fullscreen so I think yes 👀 . Unless we'd like to always do this? which is fair
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.
Ahh okay, I wasn't sure if we always wanted to do it for any ios situation 😄
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/380690 |
🟢 golden-jobs completed with status |
Relates to https://github.com/github/primer/issues/5118
iPhones force the screen to zoom in to input when the text is under 16px font size. This PR adds a css media query to make the input font size 16px on narrow screens when on full screen to prevent zoom in on SelectPanel.
Changelog
New
Rollout strategy
Testing & Reviewing
Merge checklist