Font transitions #989
Font transitions #989
Conversation
- Implement Lando Norris-style font-size transitions for interactive elements - Add font-size-based hover effects for tabs, navigation, and sidebar items - Include CSS variables for consistent transition timing and scaling - Remove hover transitions from 8knot logo, title, and subtitle - Support responsive adjustments for mobile devices - Include accessibility features for reduced motion preferences
- Add --baby-blue-500: #199AD6 to :root for focus outline color - Ensures focus states use the intended brand color - Makes font_transitions.css self-contained with all required variables
WalkthroughAdds a new CSS stylesheet that centralizes variables and extensive transition/interaction rules for UI components (tabs, nav, buttons, cards, dropdowns, forms, images, tables, sidebars), with responsive/mobile adjustments, prefers-reduced-motion and coarse-pointer fallbacks, focus states, and performance-tuning properties. Changes
Sequence Diagram(s)(omitted — changes are stylesheet-only and do not modify control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
EngCaioFonseca
moved this from Backlog
to Testing | Waiting on PR response | Blocked
in Aspen Project Board
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
8Knot/assets/font_transitions.css(1 hunks)
🔇 Additional comments (6)
8Knot/assets/font_transitions.css (6)
457-459: Confirm intent: full 180° rotation on sidebar toggle hover is intentional.Line 458 applies a full
rotate(180deg)on every hover. This is a dramatic effect—verify this is the intended design behavior and not a typo (e.g., should it berotate(var(--rotate-small))instead?).
15-73: Well‑structured and comprehensive CSS variable definitions.The extensive use of CSS custom properties for timing, scaling, transforms, shadows, and letter-spacing is excellent for maintainability and consistency. The variable naming is clear and hierarchical. Responsive adjustments in the mobile media query (lines 483–516) appropriately scale these values down, which is a best practice for performance and usability on smaller screens.
533-549: Strong accessibility provisions for motion preferences and touch devices.The
prefers-reduced-motionmedia query (lines 533–549) properly disables animations and maintains minimal opacity feedback for usability, which aligns with WCAG guidelines. The touch device media query (lines 557–572) correctly disables transform and letter-spacing on coarse pointers to prevent sticky states. These provisions are thoughtful and well-implemented.
597-611: GPU acceleration optimization is appropriate.The use of
backface-visibility: hidden,perspective, and-webkit-prefixes (lines 607–610) follows performance best practices for smooth animations and reduced repaints on modern browsers.
342-342: Verify your project's Mantine version to ensure MultiSelect data-hovered attribute compatibility.Web search confirms that Mantine Select/MultiSelect components expose a data-hovered attribute for styling item elements, so the selector at line 342 is correctly targeting the documented attribute. However, Mantine removed data-hovered from Menu.Item in the v8 migration—this does not affect MultiSelect, but you should confirm your project's Mantine version to rule out other potential breaking changes or attribute name shifts.
587-587: CSS variable--baby-blue-500is properly defined and will load correctly.The variable is defined in
8Knot/assets/color.cssline 28 (--baby-blue-500: #199AD6;) and will be available whenfont_transitions.cssis processed. In Dash applications, CSS files in the/assets/directory are auto-loaded alphabetically, socolor.cssloads beforefont_transitions.css, ensuring the variable is available. No action is needed.Likely an incorrect or invalid review comment.
| .nav-link:focus, | ||
| .sidebar-section:focus, | ||
| .landing-learn-button:focus, | ||
| .tabs-header .nav-tabs .nav-link:focus, | ||
| .page-nav-btn:focus, | ||
| .about-graph-button:focus { | ||
| transform: scale(var(--hover-scale-small)); | ||
| outline: var(--outline-width) solid var(--baby-blue-500); | ||
| outline-offset: var(--outline-offset); | ||
| } |
There was a problem hiding this comment.
Focus states are incomplete—missing coverage for hover-enabled elements.
Focus state transitions are defined only for navigation, buttons, and tabs (lines 580–589), but not for cards, images, or form inputs that have hover effects. This creates an inconsistent keyboard navigation experience and potential accessibility gaps.
Consider adding focus styles for additional interactive elements:
.nav-link:focus,
.sidebar-section:focus,
.landing-learn-button:focus,
.tabs-header .nav-tabs .nav-link:focus,
.page-nav-btn:focus,
.about-graph-button:focus {
transform: scale(var(--hover-scale-small));
outline: var(--outline-width) solid var(--baby-blue-500);
outline-offset: var(--outline-offset);
}
+ .info_card:focus,
+ .page-item:focus,
+ .feature-image:focus,
+ .instruction_image:focus {
+ outline: var(--outline-width) solid var(--baby-blue-500);
+ outline-offset: var(--outline-offset);
+ }📝 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.
| .nav-link:focus, | |
| .sidebar-section:focus, | |
| .landing-learn-button:focus, | |
| .tabs-header .nav-tabs .nav-link:focus, | |
| .page-nav-btn:focus, | |
| .about-graph-button:focus { | |
| transform: scale(var(--hover-scale-small)); | |
| outline: var(--outline-width) solid var(--baby-blue-500); | |
| outline-offset: var(--outline-offset); | |
| } | |
| .nav-link:focus, | |
| .sidebar-section:focus, | |
| .landing-learn-button:focus, | |
| .tabs-header .nav-tabs .nav-link:focus, | |
| .page-nav-btn:focus, | |
| .about-graph-button:focus { | |
| transform: scale(var(--hover-scale-small)); | |
| outline: var(--outline-width) solid var(--baby-blue-500)); | |
| outline-offset: var(--outline-offset); | |
| } | |
| .info_card:focus, | |
| .page-item:focus, | |
| .feature-image:focus, | |
| .instruction_image:focus { | |
| outline: var(--outline-width) solid var(--baby-blue-500)); | |
| outline-offset: var(--outline-offset); | |
| } |
🤖 Prompt for AI Agents
In 8Knot/assets/font_transitions.css around lines 580 to 589, the focus rules
only target nav links, buttons and tabs causing missing focus styles for other
hover-enabled interactive elements; extend the selector list to include
card-like elements, images and form controls that use hover effects (e.g. .card,
.interactive-card, img.interactive, .hoverable, input[type="text"], textarea,
select, .form-control) and apply the same transform, outline and outline-offset
properties so keyboard focus matches hover visual behavior; ensure specificity
and ordering match existing rules and that any bespoke components use their
class names to avoid over-broad selectors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
8Knot/assets/font_transitions.css (2)
37-49: Consider consolidating or removing unused CSS variables.A few variables are defined but not referenced in any transitions or hover states:
--hover-font-scale: 1.15(line 38) — only--hover-font-scale-small/medium/largeare used--shadow-small(line 55) — no selector references this shadow valueThese can be removed for clarity or kept for future use. Given the scope of this stylesheet, keeping them is reasonable if planned for future enhancements.
Also applies to: 54-61
81-93: Minor: Redundantallkeyword in transition properties.Several selectors use both
alland explicit properties in transitions, e.g., line 82:transition: all var(--transition-ease), font-size var(--transition-ease), letter-spacing var(--transition-ease), transform var(--transition-fast);Since
allalready includes all properties, the explicit properties are redundant. The durations on specific properties will override, but this could be simplified:transition: font-size var(--transition-ease), letter-spacing var(--transition-ease), transform var(--transition-fast);This is a minor code-cleanliness issue and doesn't affect functionality.
Also applies to: 101-110, 118-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
8Knot/assets/font_transitions.css(1 hunks)
⏰ 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). (2)
- GitHub Check: End-to-end test (Docker)
- GitHub Check: End-to-end test (Podman)
🔇 Additional comments (4)
8Knot/assets/font_transitions.css (4)
472-605: Excellent accessibility and performance handling.The stylesheet demonstrates strong consideration for user preferences and device capabilities:
- Reduced motion (lines 527–543): Correctly disables all transforms while maintaining visual feedback via opacity
- Touch devices (lines 551–566): Appropriate fallback that removes transforms on coarse pointers to prevent sticky hover states
- Mobile optimization (lines 477–519): Proportional scaling reductions for smoother performance on constrained devices
- GPU acceleration (lines 589–605): Proper use of
backface-visibilityandperspectivefor transform-heavy animationsThis is solid implementation.
232-284: Good semantic use of cursor properties.Titles (lines 235–277) correctly specify
cursor: defaultfor non-interactive elements, while images (lines 384, 399) appropriately usecursor: pointerfor interactive content. This provides clear visual feedback to users about element interactivity.Also applies to: 380-405
574-583: Focus states incomplete—many hover-enabled interactive elements lack focus styles.The CSS file confirms the concern: 23 elements have hover states, but only 7 have focus styles. Key interactive elements missing focus coverage include
.info_card,.page-item,.instruction_card,.search-bar-item,.custom-radio-buttons,.search-help-button,.search-list-button,.sidebar-toggle, and.sidebar-dropdown-container .collapse a.However, not all elements with hover should receive focus styles—text elements like headings (
.main-title h1,.section-title h3) and titles are non-interactive and should be excluded. Before implementing the fix:
- Verify which elements are actually keyboard-accessible in the HTML (role, tabindex, or native interactive elements)
- Add focus styles only for truly keyboard-interactive elements
- Exclude decorative images (
.feature-image,.instruction_image) unless they are keyboard-focusable
574-583: No action required—CSS variable is properly defined and accessible.The
--baby-blue-500variable is defined in8Knot/assets/color.css:28as#199AD6and is already used successfully across multiple CSS files includinglanding_page.css. CSS custom properties cascade globally once defined in:root, so the variable is accessible infont_transitions.cssat line 581. The outline styling will render correctly.Likely an incorrect or invalid review comment.
MoralCode
left a comment
MoralCode
left a comment
There was a problem hiding this comment.
This seems like a really big diff for a couple animations in what sounds like very narrowly defined areas of the UI.
Is there a way we could add these transitions to our existing CSS classes in a smaller subset of them to try it out first before animating (in some cases) every css property (some of the animations include all)
|
Running locally and something feels a little off with the user experience/look of the animations. Im not completely against the idea but am more concerned about the search bar. I think it might be a good idea to meet with jess (if anyone has her gh handle add it in) and get her perspective on it |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
8Knot/assets/font_transitions.css (2)
576-585: Incomplete focus states—keyboard navigation missing for multiple interactive elements.Focus styles are only defined for 6 elements (lines 576–585), but many other interactive components with hover effects lack keyboard focus states:
.mantine-MultiSelect-item,.search-bar-item,.custom-radio-buttons .form-check-label,.search-help-button,.search-list-button,.sidebar-toggle,.sidebar-dropdown-container .collapse a, and.dbc .table tbody tr. This creates an inconsistent keyboard navigation experience.Extend the focus rule to include all hover-enabled interactive elements:
.nav-link:focus, .sidebar-section:focus, .landing-learn-button:focus, .tabs-header .nav-tabs .nav-link:focus, .page-nav-btn:focus, .about-graph-button:focus, + .sidebar-dropdown-container .collapse a:focus, + .mantine-MultiSelect-item:focus, + .search-bar-item:focus, + .custom-radio-buttons .form-check-label:focus, + .search-help-button:focus, + .search-list-button:focus, + .sidebar-toggle:focus, + .dbc .table tbody tr:focus { transform: scale(var(--hover-scale-small)); outline: var(--outline-width) solid var(--baby-blue-500, #199ad6); outline-offset: var(--outline-offset); }
576-585: Critical: Undefined CSS variable breaks focus states.Line 583 references
--baby-blue-500which is not defined in the:rootvariables (lines 17–73). When a custom property is not defined and no fallback is provided, the property is treated as unset, causing focus outlines to disappear and breaking keyboard navigation accessibility.Add the color variable to the
:rootblock and provide a fallback in the focus rule::root { /* ... existing variables ... */ + --baby-blue-500: #199ad6; }Alternatively, add a fallback value in the focus rule:
.nav-link:focus, .sidebar-section:focus, .landing-learn-button:focus, .tabs-header .nav-tabs .nav-link:focus, .page-nav-btn:focus, .about-graph-button:focus { transform: scale(var(--hover-scale-small)); - outline: var(--outline-width) solid var(--baby-blue-500); + outline: var(--outline-width) solid var(--baby-blue-500, #199ad6); outline-offset: var(--outline-offset); }
🧹 Nitpick comments (2)
8Knot/assets/font_transitions.css (2)
275-332: Remove or document commented-out transition rules.Approximately 60 lines of commented-out CSS (card titles, info cards, page items, instruction cards, feature images, instruction images) should either be removed entirely or documented with comments explaining why they're disabled. Dead code creates maintenance burden and confusion about intended functionality.
Given the PR feedback that animations feel "a little off," particularly around the search bar, clarify whether these rules should be:
- Removed permanently (if no longer needed), or
- Re-enabled with UX refinements (if planned for future use)
If removing, apply:
- /* .card h3.card-title { - transition: letter-spacing var(--transition-smooth), transform var(--transition-smooth), color var(--transition-fast); - transform-origin: left center; - cursor: default; - } - - .card h3.card-title:hover { - letter-spacing: var(--letter-spacing-large); - transform: scale(var(--hover-scale-tiny)); - } */If keeping, add documentation:
+ /* Card titles, info cards, and image transitions disabled per UX review. + Re-enable when hover animation effects have been refined for better UX. */ - /* .card h3.card-title { ... */
593-603: Monitor GPU memory usage from extensivewill-changedeclarations.The stylesheet applies
will-change: transformto many elements (nav links, sidebar, buttons, tabs, etc., lines 593–603 and throughout). While this enables GPU acceleration, excessive use can increase GPU memory consumption. This is generally acceptable for most devices but should be tested on lower-end devices and older tablets to ensure performance remains smooth.Consider selectively disabling
will-changeon touch devices if performance issues arise:@media (hover: none) and (pointer: coarse) { /* ... existing reset rules ... */ + * { + will-change: auto; + } }This is optional—only implement if you observe performance degradation on touch devices during testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
8Knot/assets/font_transitions.css(1 hunks)
⏰ 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). (2)
- GitHub Check: End-to-end test (Docker)
- GitHub Check: End-to-end test (Podman)
🔇 Additional comments (1)
8Knot/assets/font_transitions.css (1)
354-362: Review search bar animations per reviewer feedback.The PR reviewer (cdolfi) specifically flagged that animations feel "a little off" and expressed concern about the search bar. Current
.search-bar-itemtransitions useallwith both scale and horizontal translate, which could feel unnatural if not carefully tuned.Consider reviewing the search bar hover behavior with your design/UX team. Potential refinements:
- Reduce
--hover-scale-mediumfor search items (currently 1.02 desktop) if the zoom feels jarring- Simplify to a single transform property (scale OR translate) rather than combining both
- Consider disabling hover effects on search items if they conflict with dropdown/list interactions
.search-bar-item { - transition: all var(--transition-fast), transform var(--transition-fast); + transition: transform var(--transition-fast), background-color var(--transition-fast); transform-origin: left center; will-change: transform; } .search-bar-item:hover { - transform: scale(var(--hover-scale-medium)) translateX(var(--hover-slide-medium)); + transform: translateX(var(--hover-slide-small)); }
| @media (hover: none) and (pointer: coarse) { | ||
| /* Reset transforms but keep background/color changes */ | ||
| .nav-link:hover, | ||
| .sidebar-section:hover, | ||
| .landing-learn-button:hover, | ||
| .tabs-header .nav-tabs .nav-link:hover, | ||
| .page-nav-btn:hover, | ||
| .about-graph-button:hover { | ||
| transform: none; | ||
| letter-spacing: initial; | ||
| } | ||
| } |
There was a problem hiding this comment.
Incomplete touch device adjustments—sticky hover states on unsupported elements.
The touch device media query (lines 557–568) only resets hover effects for 6 elements, but many other interactive components with transforms still have will-change: transform and hover rules active on touch devices. This creates sticky or unresponsive hover states on tablets and touch-enabled laptops.
Elements not covered include: .sidebar-dropdown-container .collapse a, .landing-tab, .mantine-MultiSelect-item, .search-bar-item, .custom-radio-buttons .form-check-label, .search-help-button, .search-list-button, .sidebar-toggle, and .dbc .table tbody tr.
Extend the touch device media query to reset all hover-enabled elements:
@media (hover: none) and (pointer: coarse) {
.nav-link:hover,
.sidebar-section:hover,
.landing-learn-button:hover,
.tabs-header .nav-tabs .nav-link:hover,
.page-nav-btn:hover,
.about-graph-button:hover,
+ .sidebar-dropdown-container .collapse a:hover,
+ .landing-tab:hover,
+ .mantine-MultiSelect-item[data-hovered],
+ .search-bar-item:hover,
+ .custom-radio-buttons .form-check:hover .form-check-label,
+ .search-help-button:hover,
+ .search-list-button:hover,
+ .sidebar-toggle:hover,
+ .dbc .table tbody tr:hover {
transform: none;
letter-spacing: initial;
}
}📝 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.
| @media (hover: none) and (pointer: coarse) { | |
| /* Reset transforms but keep background/color changes */ | |
| .nav-link:hover, | |
| .sidebar-section:hover, | |
| .landing-learn-button:hover, | |
| .tabs-header .nav-tabs .nav-link:hover, | |
| .page-nav-btn:hover, | |
| .about-graph-button:hover { | |
| transform: none; | |
| letter-spacing: initial; | |
| } | |
| } | |
| @media (hover: none) and (pointer: coarse) { | |
| /* Reset transforms but keep background/color changes */ | |
| .nav-link:hover, | |
| .sidebar-section:hover, | |
| .landing-learn-button:hover, | |
| .tabs-header .nav-tabs .nav-link:hover, | |
| .page-nav-btn:hover, | |
| .about-graph-button:hover, | |
| .sidebar-dropdown-container .collapse a:hover, | |
| .landing-tab:hover, | |
| .mantine-MultiSelect-item[data-hovered], | |
| .search-bar-item:hover, | |
| .custom-radio-buttons .form-check:hover .form-check-label, | |
| .search-help-button:hover, | |
| .search-list-button:hover, | |
| .sidebar-toggle:hover, | |
| .dbc .table tbody tr:hover { | |
| transform: none; | |
| letter-spacing: initial; | |
| } | |
| } |
🤖 Prompt for AI Agents
In 8Knot/assets/font_transitions.css around lines 557–568, the touch-device
media query currently resets hover for only six selectors, causing sticky hover
states on many other interactive elements; extend this media query to include
the missing selectors (.sidebar-dropdown-container .collapse a, .landing-tab,
.mantine-MultiSelect-item, .search-bar-item, .custom-radio-buttons
.form-check-label, .search-help-button, .search-list-button, .sidebar-toggle,
.dbc .table tbody tr) and for each selector set transform: none; letter-spacing:
initial; will-change: auto; and disable hover-only transitions (e.g.,
transition: none or remove hover-specific transition properties) so touch
devices no longer get stuck on hover states.
3 participants
This PR adds dynamic transition to the options on the top nav bar and sidebar when the user hovers the cursor over them.
Summary by CodeRabbit