Fix auto-zoom on iOS devices #1029
Conversation
WalkthroughReplaces Tailwind Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Security recommendations
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,vue}⚙️ CodeRabbit configuration file
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/components/ui/command/CommandInput.vue(1 hunks)frontend/components/ui/input/Input.vue(1 hunks)frontend/components/ui/select/SelectTrigger.vue(1 hunks)frontend/components/ui/tags-input/TagsInputInput.vue(1 hunks)frontend/components/ui/textarea/Textarea.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,vue}
⚙️ CodeRabbit configuration file
**/*.{ts,vue}: Check for hardcoded strings in UI components that should be translatable.
Look for:
- String literals in Vue components (e.g. Click me)
- Alert messages, error messages, and user-facing text
- Placeholder text and labels
Files:
frontend/components/ui/tags-input/TagsInputInput.vuefrontend/components/ui/command/CommandInput.vuefrontend/components/ui/select/SelectTrigger.vuefrontend/components/ui/textarea/Textarea.vuefrontend/components/ui/input/Input.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Backend Server Tests / Go
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
🔇 Additional comments (1)
frontend/components/ui/input/Input.vue (1)
27-29: Explicitly settext-baseto guarantee the 16 px targetDropping
text-smlets the input inherit whatever font size the parent uses. In several screens we wrap form controls intext-smcontainers, so the element will still compute to 14 px and iOS will continue to auto-zoom. Please set the input’s class to include an explicit 16 px size (e.g. Tailwind’stext-base) so we meet the stated goal everywhere.- 'flex h-10 w-full rounded-md border border-input bg-background px-3 py-2 ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-muted-foreground placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', + 'flex h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-base ring-offset-background file:border-0 file:bg-transparent file:text-base file:font-medium file:text-muted-foreground placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50',Likely an incorrect or invalid review comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/components/ui/tags-input/TagsInputInput.vue (2)
18-18: Verify visible focus indication
focus:outline-noneis present. Confirm the wrapper supplies a visible focus style (ring/border) for keyboard users; otherwise add afocus-visibleindicator here.Example:
- :class="cn('min-h-6 focus:outline-none flex-1 bg-transparent px-1', props.class, 'text-base')" + :class="cn('min-h-6 focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1 flex-1 bg-transparent px-1', props.class, 'text-base')"
18-18: Security noteNo new attack surface. Keep
props.classstrictly internal (not user-provided) to avoid style injection surprises, and avoid addinguser-scalable=noin meta viewport (accessibility and security ergonomics).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/components/ui/command/CommandInput.vue(1 hunks)frontend/components/ui/select/SelectTrigger.vue(1 hunks)frontend/components/ui/tags-input/TagsInputInput.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/components/ui/select/SelectTrigger.vue
- frontend/components/ui/command/CommandInput.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,vue}
⚙️ CodeRabbit configuration file
**/*.{ts,vue}: Check for hardcoded strings in UI components that should be translatable.
Look for:
- String literals in Vue components (e.g. Click me)
- Alert messages, error messages, and user-facing text
- Placeholder text and labels
Files:
frontend/components/ui/tags-input/TagsInputInput.vue
🔇 Additional comments (1)
frontend/components/ui/tags-input/TagsInputInput.vue (1)
18-18: i18n checkNo user-facing strings introduced; nothing to translate here. LGTM.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| :class="cn( | ||
| 'min-h-6 focus:outline-none flex-1 bg-transparent px-1', | ||
| props.class, | ||
| 'text-base' |
There was a problem hiding this comment.
This is at the end because Coderrabbit suggested it in case props.class sets the font-size again.
There was a problem hiding this comment.
I dont think the text size is overwritten anywhere tagsinputinput is used and i think it should be at the start so it can be overwritten by props
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/components/ui/input/Input.vue(1 hunks)frontend/components/ui/select/SelectTrigger.vue(1 hunks)frontend/components/ui/tags-input/TagsInputInput.vue(1 hunks)frontend/components/ui/textarea/Textarea.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/components/ui/select/SelectTrigger.vue
- frontend/components/ui/textarea/Textarea.vue
- frontend/components/ui/tags-input/TagsInputInput.vue
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,vue}
⚙️ CodeRabbit configuration file
**/*.{ts,vue}: Check for hardcoded strings in UI components that should be translatable.
Look for:
- String literals in Vue components (e.g. Click me)
- Alert messages, error messages, and user-facing text
- Placeholder text and labels
Files:
frontend/components/ui/input/Input.vue
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 4/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 3/4
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 2/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 17
- GitHub Check: End-to-End Playwright Tests / E2E Playwright Testing 1/4
- GitHub Check: Frontend Tests / Integration Tests PGSQL 15
- GitHub Check: Frontend Tests / Integration Tests PGSQL 16
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/arm/v7)
- GitHub Check: build (linux/arm64)
- GitHub Check: build (linux/amd64)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
I'm baffled as to why this is failing, I'll take a look at it again this evening. |
|
If failing you mean the integration test with SQLite, it's a known issue, and one we basically ignore so long as it's the specific error we regularly see from it. |
|
@tankerkiller125 Looks like it was that one- I rebased and it's fine now. |
|
LGTM but I'd value a second opinion. @tonyaellie ? :) |
|
will review tomorrow :) |
I would have thought that would cause issues on iPads, but it turns out it doesn't for some reason. Maybe iPad OS doesn't have the auto-zoom behavior. However, it does keep the auto-zoom in place on iPhones in landscape, so I'm inclined to keep this. The alternative seems to be to prevent users from being able to zoom altogether but that seems problematic in its own way. |
|
Alternatively you could add a check for iPhones and if so enable it |
|
We could do something like this: default.vue <div id="app" class="group/iphone" :data-iphone="isIphone">const isIphone = computed(
() => navigator && (/iPhone/i.test(navigator.userAgent) || /iPhone/i.test(navigator.platform))
);input text |
|
My concern there is that device detection is inherently fragile, evolving over time and creating maintenance cost. It also looks like that suggestion would leverage some sort of runtime detection, possibly causing a reflow after the page has been rendered and impacting performance/user experience. For a feature that runs on every page load and affects every form input, these costs can add up. I think the root problem is that the input text is too small by default and Apple/Android's differing handling of that shouldn't be the thing we target in a solution. If you look around the web, this is a common problem and making the text bigger seems to be the agreed upon solution. |
4 participants
What type of PR is this?
What this PR does / why we need it:
Currently, when using Homebox on iOS, tapping ton an input field zooms in and forces users to pan around the page or zoom out again. This is caused by the input font size being less than 16px. This PR increases all input areas to 16px to prevent this. Another approach involved changing the
meta viewporttag, but that would have been worse for accessibility, preventing users from zooming altogether.Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
I couldn't actually test this on my device comprehensively as it doesn't seem possible to access a development backend from a different device and I'm primarily a front-end developer.
Frontend CI is failing, but that's fixed in #1028, as it's unrelated to this PR
Testing
task go:runandtask ui:devwith some changes to hosts allowed me to open this on my iPhone and see the login screen, verifying that it was fixed. Otherwise, this is a Claude Code run of the codebase to cleartext-smon all inputs.Summary by CodeRabbit