-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ui): align map UI with interface guidelines #40
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
base: feature/ui-test-documentation
Are you sure you want to change the base?
Conversation
Deploying veeam-data-cloud-services-map with
|
| Latest commit: |
45c622c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://91d95004.veeam-data-cloud-services-map.pages.dev |
| Branch Preview URL: | https://feature-ui-guideline-fixes.veeam-data-cloud-services-map.pages.dev |
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
Aligns the map UI’s metadata, theming behavior, motion settings, and accessibility with interface guidelines.
Changes:
- Added/updated document metadata (description, theme-color, color-scheme) and synced
theme-colorwith runtime theme changes. - Replaced several
transition: allusages with narrower transitions and added aprefers-reduced-motionCSS override. - Improved a11y for search/filter controls and made search results interactive elements.
| border-bottom: 1px solid #334155; | ||
| background: transparent; | ||
| border: none; | ||
| width: 100%; | ||
| text-align: left; |
Copilot
AI
Jan 28, 2026
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.
In .search-result-item, border-bottom: 1px solid ... is immediately overridden by border: none, so the intended separator line will never render. Remove the shorthand border reset or reorder declarations so the bottom border remains (while still clearing the button’s default border).
| border-bottom: 1px solid #334155; | |
| background: transparent; | |
| border: none; | |
| width: 100%; | |
| text-align: left; | |
| background: transparent; | |
| border: none; | |
| width: 100%; | |
| text-align: left; | |
| border-bottom: 1px solid #334155; |
| <button type="button" class="search-result-item" data-index="${index}" id="search-result-${index}" role="option" aria-selected="false"> | ||
| <span class="provider-dot" style="background-color: ${providerColor}"></span> |
Copilot
AI
Jan 28, 2026
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.
renderSearchResults() outputs a <button> with role="option". In ARIA-in-HTML this role is not permitted on a button, and it can trigger automated a11y violations. Prefer using a non-interactive element for role="option" (and keep focus on the combobox via aria-activedescendant), or keep <button> semantics and adjust the popup pattern/roles accordingly.
| scroll-behavior: auto !important; | ||
| } | ||
| } | ||
| </style> |
Copilot
AI
Jan 28, 2026
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.
The new prefers-reduced-motion CSS block only short-circuits CSS transitions/animations, but key UI motion is still driven by JS (e.g., Leaflet map.setView(..., { animate: true })). To fully honor reduced motion, consider checking matchMedia('(prefers-reduced-motion: reduce)') and disabling Leaflet pan/zoom animations and any other JS-driven motion when enabled.
| </style> | |
| </style> | |
| <script> | |
| (function () { | |
| if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') { | |
| return; | |
| } | |
| var prefersReducedMotionQuery = window.matchMedia('(prefers-reduced-motion: reduce)'); | |
| function applyReducedMotionPreference() { | |
| var reduce = prefersReducedMotionQuery.matches; | |
| window.prefersReducedMotion = reduce; | |
| if (!window.L || !L.Map || typeof L.Map.mergeOptions !== 'function') { | |
| return; | |
| } | |
| L.Map.mergeOptions({ | |
| fadeAnimation: !reduce, | |
| zoomAnimation: !reduce, | |
| markerZoomAnimation: !reduce, | |
| inertia: !reduce | |
| }); | |
| } | |
| if (typeof prefersReducedMotionQuery.addEventListener === 'function') { | |
| prefersReducedMotionQuery.addEventListener('change', applyReducedMotionPreference); | |
| } else if (typeof prefersReducedMotionQuery.addListener === 'function') { | |
| prefersReducedMotionQuery.addListener(applyReducedMotionPreference); | |
| } | |
| if (typeof window.addEventListener === 'function') { | |
| window.addEventListener('load', applyReducedMotionPreference); | |
| } else { | |
| applyReducedMotionPreference(); | |
| } | |
| })(); | |
| </script> |
| </div> | ||
|
|
||
| <button id="resetFilters" class="hidden items-center gap-1.5 px-2.5 py-1.5 rounded-lg bg-amber-500/20 light:bg-amber-100 border border-amber-500/40 light:border-amber-400 text-amber-400 light:text-amber-700 hover:bg-amber-500/30 light:hover:bg-amber-200 text-xs font-medium transition-all duration-200 flex-shrink-0" title="Reset all filters"> | ||
| <button id="resetFilters" aria-label="Reset all filters" class="hidden items-center gap-1.5 px-2.5 py-1.5 rounded-lg bg-amber-500/20 light:bg-amber-100 border border-amber-500/40 light:border-amber-400 text-amber-400 light:text-amber-700 hover:bg-amber-500/30 light:hover:bg-amber-200 text-xs font-medium transition-all duration-200 flex-shrink-0" title="Reset all filters"> |
Copilot
AI
Jan 28, 2026
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 button still uses Tailwind transition-all, which reintroduces the “transition everything” behavior the PR is otherwise removing. Consider narrowing it to the specific properties that change here (e.g., colors/border colors) to avoid animating layout-affecting properties.
| <button id="resetFilters" aria-label="Reset all filters" class="hidden items-center gap-1.5 px-2.5 py-1.5 rounded-lg bg-amber-500/20 light:bg-amber-100 border border-amber-500/40 light:border-amber-400 text-amber-400 light:text-amber-700 hover:bg-amber-500/30 light:hover:bg-amber-200 text-xs font-medium transition-all duration-200 flex-shrink-0" title="Reset all filters"> | |
| <button id="resetFilters" aria-label="Reset all filters" class="hidden items-center gap-1.5 px-2.5 py-1.5 rounded-lg bg-amber-500/20 light:bg-amber-100 border border-amber-500/40 light:border-amber-400 text-amber-400 light:text-amber-700 hover:bg-amber-500/30 light:hover:bg-amber-200 text-xs font-medium transition-colors duration-200 flex-shrink-0" title="Reset all filters"> |
Summary
Testing