Feat/responsive - #1060
Conversation
…ted under explore
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (61)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Coverage Report
File CoverageNo changed files found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f6928b6e-2c50-4134-9e77-a104f3a92bf1
📒 Files selected for processing (61)
app/components/device-detail/device-detail-box.tsxapp/components/device-detail/graph.tsxapp/components/device/new/custom-device-config.tsxapp/components/device/new/general-info.tsxapp/components/device/new/location-info.tsxapp/components/device/new/new-device-stepper.tsxapp/components/error-message.tsxapp/components/header/download.tsxapp/components/header/home/index.tsxapp/components/header/info/index.tsxapp/components/header/menu/index.tsxapp/components/header/nav-bar/index.tsxapp/components/landing/footer.tsxapp/components/landing/header/header.tsxapp/components/landing/header/language-selector.tsxapp/components/landing/header/theme-toggle.tsxapp/components/landing/sections/connect.tsxapp/components/landing/sections/features.tsxapp/components/landing/sections/integrations.tsxapp/components/landing/sections/partners.tsxapp/components/landing/sections/pricing-plans.tsxapp/components/landing/stats.tsxapp/components/language-select.tsxapp/components/map/legend.tsxapp/components/map/topbar.tsxapp/components/markdown-content.tsxapp/components/mydevices/dt/data-table.tsxapp/components/nav-bar.tsxapp/components/search/search-list-item.tsxapp/components/search/search-list.tsxapp/components/theme-select.tsxapp/components/ui/alert-dialog.tsxapp/components/ui/dialog.tsxapp/components/ui/drawer.tsxapp/components/ui/sheet.tsxapp/components/ui/table.tsxapp/components/ui/tabs.tsxapp/components/ui/toast.tsxapp/components/viewport-portal.tsxapp/root.tsxapp/routes/about.tsxapp/routes/account.password-reset.tsxapp/routes/device.$deviceId.dataupload.tsxapp/routes/device.$deviceId.edit.security.tsxapp/routes/device.$deviceId.edit.sensors.tsxapp/routes/device.$deviceId.edit.transfer.tsxapp/routes/device.$deviceId.edit.tsxapp/routes/device.dashboard.$deviceId.tsxapp/routes/device.new.tsxapp/routes/device.transfer.tsxapp/routes/docs.tsxapp/routes/explore.forgot.tsxapp/routes/explore.login.tsxapp/routes/explore.register.tsxapp/routes/imprint.tsxapp/routes/privacy.tsxapp/routes/profile.$username.tsxapp/routes/settings.account.tsxapp/routes/settings.preferences.tsxapp/routes/settings.profile.tsxapp/routes/settings.tsx
| className="inline-flex size-11 items-center justify-center rounded-lg p-2 text-sm text-gray-500 hover:bg-gray-100 focus:ring-2 focus:ring-gray-200 focus:outline-hidden lg:hidden" | ||
| aria-controls="navbar-cta" | ||
| aria-expanded="false" | ||
| aria-expanded={openMenu} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Point aria-controls at the rendered mobile menu.
aria-expanded={openMenu} now follows the state, but aria-controls="navbar-cta" still names the desktop navigation element. That element is hidden below lg. The visible mobile menu is the separate element rendered at Line 110 and has no matching id. Assistive technology receives a false control relationship.
Give the mobile menu its own id, use that id in aria-controls, reference the actual trigger from aria-labelledby, and update the hidden label at Line 95 when openMenu is true.
Proposed ARIA fix
<button
+ id="navbar-menu-toggle"
...
- data-collapse-toggle="navbar-cta"
+ data-collapse-toggle="navbar-mobile-menu"
...
- aria-controls="navbar-cta"
+ aria-controls="navbar-mobile-menu"
aria-expanded={openMenu}
>
- <span className="sr-only">Open main menu</span>
+ <span className="sr-only">
+ {openMenu ? 'Close main menu' : 'Open main menu'}
+ </span>
...
<div
+ id="navbar-mobile-menu"
...
- aria-labelledby="options-menu"
+ aria-labelledby="navbar-menu-toggle"
>| <div className="mt-4 flex flex-col justify-center gap-3 sm:flex-row sm:gap-5"> | ||
| <div id="left" className="flex w-full flex-col gap-3 sm:w-auto"> | ||
| <a | ||
| href="http://github.com/openSenseMap/frontend" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use an HTTPS repository URL.
The changed href uses http://github.com/openSenseMap/frontend. Use the HTTPS URL directly instead of relying on redirects or HSTS behavior.
Proposed fix
- href="http://github.com/openSenseMap/frontend"
+ href="https://github.com/openSenseMap/frontend"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| href="http://github.com/openSenseMap/frontend" | |
| href="https://github.com/openSenseMap/frontend" |
| size="topbarPill" | ||
| className="absolute top-14 left-0 h-11 px-3 lg:static lg:h-10" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the shared topbar variant on large screens.
When lg:static returns the “My Area” button to the desktop header row, omitting variant="topbar" selects the Button default instead of the shared topbar surface. The Button implementation defines these as different variants, while the adjacent controls keep variant="topbar". (raw.githubusercontent.com)
Keep the topbar variant. Use the responsive classes only for placement and size. If a different mobile appearance is required, override it explicitly below lg.
Proposed fix
<Button
type="button"
+ variant="topbar"
size="topbarPill"
className="absolute top-14 left-0 h-11 px-3 lg:static lg:h-10"Source: MCP tools
|
|
||
| const searchListItemStyle = cva( | ||
| 'data-[active=true]:bg-light-green relative my-1 flex h-8 items-center gap-2 rounded-lg px-2 data-[active=true]:text-white', | ||
| 'data-[active=true]:bg-light-green relative my-1 flex min-h-11 cursor-pointer items-center gap-2 rounded-lg px-2 data-[active=true]:text-white md:h-8 md:min-h-0', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep mobile result targets until the desktop breakpoint.
Line 20 reduces result rows to 32px at md. The search panel remains in its mobile layout until lg. Touch users between 768px and 1023px therefore get smaller result targets. Use lg for the compact row height.
Proposed fix
- '... min-h-11 ... md:h-8 md:min-h-0'
+ '... min-h-11 ... lg:h-8 lg:min-h-0'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'data-[active=true]:bg-light-green relative my-1 flex min-h-11 cursor-pointer items-center gap-2 rounded-lg px-2 data-[active=true]:text-white md:h-8 md:min-h-0', | |
| 'data-[active=true]:bg-light-green relative my-1 flex min-h-11 cursor-pointer items-center gap-2 rounded-lg px-2 data-[active=true]:text-white lg:h-8 lg:min-h-0', |
| const { t } = useTranslation('landing') | ||
|
|
||
| const isDesktop = useMediaQuery('(min-width: 768px)') | ||
| const showGlobe = useMediaQuery('(min-width: 1120px)') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C4 'useMediaQuery|showGlobe|GlobeComponent' app/routes/about.tsx
fd -H -t f -E node_modules -E .git . -x rg -n -C2 'ssr|serverBuild|prerender' {} || trueRepository: openSenseMap/frontend
Length of output: 1421
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- about route ---'
sed -n '80,220p' app/routes/about.tsx
printf '%s\n' '--- globe component ---'
globe_file="$(fd -H -t f -E node_modules -E .git -i 'globe.client*' . | head -n 1)"
if [ -n "$globe_file" ]; then
sed -n '1,240p' "$globe_file"
else
printf '%s\n' 'globe.client file not found'
fi
printf '%s\n' '--- SSR configuration candidates ---'
fd -H -t f -E node_modules -E .git . \
| while IFS= read -r file; do
case "$file" in
*config*|*package.json|*vite*|*entry*|*root*|*route*)
if rg -n -q 'ssr|serverBuild|prerender|hydrate|useMediaQuery|GlobeComponent' "$file" 2>/dev/null; then
printf '%s\n' "### $file"
rg -n -C2 'ssr|serverBuild|prerender|hydrate|useMediaQuery|GlobeComponent' "$file"
fi
;;
esac
done
printf '%s\n' '--- Mantine dependency versions ---'
for file in package.json package-lock.json pnpm-lock.yaml yarn.lock bun.lockb; do
if [ -f "$file" ]; then
printf '%s\n' "### $file"
rg -n -C2 '`@mantine/hooks`|mantine' "$file" | head -80
fi
doneRepository: openSenseMap/frontend
Length of output: 8850
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
import re
url = "https://unpkg.com/@mantine/hooks@9.4.2/lib/use-media-query/use-media-query.js"
source = urlopen(url, timeout=10).read().decode()
print(source)
print("\n--- relevant behavior ---")
for pattern in (
r"useState\([^;]+",
r"useEffect\([^;]+",
r"matchMedia",
r"getInitialValueInEffect",
):
print(f"{pattern}: {bool(re.search(pattern, source))}")
PYRepository: openSenseMap/frontend
Length of output: 2445
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
curl -fsSLk 'https://unpkg.com/@mantine/hooks@9.4.2/lib/use-media-query/use-media-query.js' \
| sed -n '1,220p'Repository: openSenseMap/frontend
Length of output: 261
🌐 Web query:
Mantine @mantine/hooks 9.4.2 useMediaQuery source getInitialValueInEffect SSR
💡 Result:
In Mantine's @mantine/hooks library, getInitialValueInEffect is a configuration option available in the useMediaQuery hook that dictates how the hook determines its initial return value when the application is rendered [1][2]. By default, getInitialValueInEffect is set to true [3]. When enabled (true), the hook will return the initial value provided as the second argument (or false if none is provided) during the initial render [1][3]. It then updates to the actual media query result inside a useEffect hook after the component has mounted on the client [3]. This behavior is designed to prevent hydration mismatches in server-side rendering (SSR) environments, where the server cannot access the window.matchMedia API [1][4]. If you set getInitialValueInEffect to false, the hook will attempt to evaluate the media query immediately during the initial render [1][3]. If window.matchMedia is available (i.e., on the client side), it will return the real result immediately [3]. However, if used in an SSR environment, this can lead to hydration mismatches because the server-rendered HTML will not match the initial state calculated by the client [1][5]. For most SSR applications (like Next.js), it is recommended to keep getInitialValueInEffect as true (the default) if the hook's output is used in the rendered markup to ensure consistency between the server-rendered HTML and the initial client-side hydration [1][4][6]. If your application does not use SSR, or if the hook's value is not used in the initial rendered output (e.g., used only for event listeners or non-SSR components), you may safely set it to false to get an immediate value [1][5][7]. top_results: [1][3][4][2][7][6]
Citations:
- 1: https://mantine.dev/hooks/use-media-query/
- 2: https://v7.mantine.dev/hooks/use-media-query/
- 3: https://github.com/mantinedev/mantine/blob/master/packages/@mantine/hooks/src/use-media-query/use-media-query.ts
- 4: Hydration mismatch after SSR using useMediaQuery mantinedev/mantine#1230
- 5: https://mantine.dev/styles/responsive/
- 6: Hydration failed in nextjs when using useLocalStorage hook mantinedev/mantine#1595
- 7: https://help.mantine.dev/q/local-storage-effect
Reserve the globe space during SSR.
On desktop, useMediaQuery('(min-width: 1120px)') adds GlobeComponent after hydration. This changes the hero layout and causes layout shift. Render a stable 500px container and control visibility with CSS, or reserve the space during client-only mounting.
| <Input | ||
| id="username" | ||
| placeholder={t('enter_username')} | ||
| ref={usernameRef} | ||
| name="username" | ||
| type="text" | ||
| autoFocus={true} | ||
| /> | ||
| <p className="text-muted-foreground text-xs"> | ||
| {t('username_hint')} | ||
| </p> | ||
| {actionErrors?.username && ( | ||
| <div | ||
| className="mt-1 text-sm text-red-500" | ||
| id="password-error" | ||
| > | ||
| {t(actionErrors?.username)} | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Associate the username error with the username input.
Line 328 assigns password-error to the username error. The username Input does not set aria-invalid or aria-describedby. Screen readers cannot identify the validation error for the focused username field.
Proposed fix
<Input
id="username"
+ aria-invalid={actionErrors?.username ? true : undefined}
+ aria-describedby="username-error"
placeholder={t('enter_username')}
ref={usernameRef}
name="username"
type="text"
autoFocus={true}
/>
...
-<div className="mt-1 text-sm text-red-500" id="password-error">
+<div className="mt-1 text-sm text-red-500" id="username-error">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Input | |
| id="username" | |
| placeholder={t('enter_username')} | |
| ref={usernameRef} | |
| name="username" | |
| type="text" | |
| autoFocus={true} | |
| /> | |
| <p className="text-muted-foreground text-xs"> | |
| {t('username_hint')} | |
| </p> | |
| {actionErrors?.username && ( | |
| <div | |
| className="mt-1 text-sm text-red-500" | |
| id="password-error" | |
| > | |
| {t(actionErrors?.username)} | |
| </div> | |
| <Input | |
| id="username" | |
| aria-invalid={actionErrors?.username ? true : undefined} | |
| aria-describedby="username-error" | |
| placeholder={t('enter_username')} | |
| ref={usernameRef} | |
| name="username" | |
| type="text" | |
| autoFocus={true} | |
| /> | |
| <p className="text-muted-foreground text-xs"> | |
| {t('username_hint')} | |
| </p> | |
| {actionErrors?.username && ( | |
| <div | |
| className="mt-1 text-sm text-red-500" | |
| id="username-error" | |
| > | |
| {t(actionErrors?.username)} | |
| </div> |
| <div className="flex flex-col items-start justify-between gap-3 py-4 last:pb-0 sm:flex-row sm:items-center sm:gap-4"> | ||
| <div className="flex min-w-0 items-start gap-2"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- changed files ---'
git diff --stat
printf '%s\n' '--- target file ---'
sed -n '620,690p' app/routes/settings.preferences.tsx
printf '%s\n' '--- related checkbox and layout code ---'
sed -n '500,575p' app/routes/settings.preferences.tsx
rg -n -C 5 'newsletter|checkbox|pending|items-start|min-w-0|w-full|sm:w-auto' app/routes/settings.preferences.tsx app/components/language-select.tsxRepository: openSenseMap/frontend
Length of output: 24843
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- component definitions ---'
rg -n -C 8 'export (function|const) Label|forwardRef|function Label|export (function|const) CardContent' app/components/ui/label.tsx app/components/ui/card.tsx
printf '%s\n' '--- utility and responsive configuration ---'
rg -n -C 3 'tailwind|tailwindcss|container|screens|theme' package.json tailwind.config.* app/styles* 2>/dev/null || true
printf '%s\n' '--- newsletter translations ---'
rg -n -C 3 'receive_newsletter_messages|newsletter_confirmation_pending' . --glob '!node_modules' --glob '!build' --glob '!dist'
printf '%s\n' '--- relevant source and git state ---'
git status --short
git diff -- app/routes/settings.preferences.tsxRepository: openSenseMap/frontend
Length of output: 10582
Keep the newsletter controls within the mobile width.
At Line 640, items-start prevents the checkbox group from stretching in the column layout. Add w-full sm:w-auto to the group so long localized labels wrap within the card.
1 participant
Type of Change
Implementation
Checklist
devbranchAdditional Information