refactor: replace use of LumoUtility properties #8
Conversation
WalkthroughReplaces Lumo utility classes with internal CSS classes and adds a component-level CSS import. Introduces a variadic ChipGroup constructor. Adds a new stylesheet. Removes a test AppShellConfigurator and theme.json. Updates demos by removing alignment and sizing calls. No core logic changes beyond constructor addition and styling refactor. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/BinderDemo.java (1)
150-152: Restore theAlignmentimport or fully qualify it.
Alignmentis still referenced at Line 151, but the corresponding import (com.vaadin.flow.component.orderedlayout.FlexComponent.Alignment) disappeared. This file no longer compiles. Please add the import back (or qualify the reference).+import com.vaadin.flow.component.orderedlayout.FlexComponent.Alignment;
🧹 Nitpick comments (1)
src/main/resources/META-INF/frontend/styles/fc-recurrent-schedule-field.css (1)
14-14: Consider standardizing CSS formatting for consistency.The CSS has inconsistent spacing before colons (e.g.,
background : initialvs.background: var(--lumo-primary-color-50pct)). While not a functional issue, standardizing the format improves maintainability.Apply consistent spacing by removing spaces before colons:
-.fc-rsf-unselected { - background : initial -} +.fc-rsf-unselected { + background: initial; +}.fc-rsf-linespan { - display : inline-flex; - align-items : center; - justify-content : center; + display: inline-flex; + align-items: center; + justify-content: center;- color : var(--lumo-secondary-text-color); + color: var(--lumo-secondary-text-color);Also applies to: 19-21, 42-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ui/ChipGroup.java(3 hunks)src/main/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ui/RecurrentScheduleField.java(10 hunks)src/main/resources/META-INF/frontend/styles/fc-recurrent-schedule-field.css(1 hunks)src/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.java(0 hunks)src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/BinderDemo.java(2 hunks)src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ComponentDemo.java(1 hunks)src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ConstrainedDemo.java(0 hunks)src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/RecurrentScheduleFieldDemoView.java(0 hunks)src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/StatesDemo.java(0 hunks)src/test/resources/META-INF/frontend/themes/recurrentschedulefield/theme.json(0 hunks)
💤 Files with no reviewable changes (5)
- src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/StatesDemo.java
- src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/RecurrentScheduleFieldDemoView.java
- src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ConstrainedDemo.java
- src/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.java
- src/test/resources/META-INF/frontend/themes/recurrentschedulefield/theme.json
🔇 Additional comments (7)
src/test/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ComponentDemo.java (1)
34-35: LGTM! Alignment removal aligns with PR objectives.The removal of
AlignItems.CENTERis consistent with the PR's goal to eliminate Lumo utility class dependencies. The demo will render without centered alignment, which appears intentional as part of the broader styling refactor.src/main/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ui/ChipGroup.java (2)
54-59: LGTM! Convenient variadic constructor addition.The new variadic constructor provides a cleaner initialization API for creating chip groups with multiple chips, while properly delegating to the existing setup logic.
46-46: LGTM! CSS class migration completed correctly.The replacement of Lumo utility classes with custom
fc-rsf-*classes is consistent throughout the component. All referenced classes are defined in the new CSS file, and the read-only state styling has been properly adapted to use the new class names.Also applies to: 107-107, 112-112, 173-180
src/main/java/com/flowingcode/vaadin/addons/recurrentschedulefield/ui/RecurrentScheduleField.java (3)
64-64: LGTM! CSS import correctly added.The
@CssImportannotation properly references the new component stylesheet, enabling the custom CSS classes throughout the component.
22-28: LGTM! Standard library imports added.The additional imports for
DayOfWeek,Duration,LocalDate,LocalTime,List,Locale, andSetare standard Java types that appear to have been reorganized or made explicit. These imports are appropriate for the component's functionality.
149-153: LGTM! Comprehensive CSS class migration.The replacement of Lumo utility classes with custom
fc-*prefixed classes is systematic and consistent across all layout components (date selectors, days selectors, time selectors, and root layouts). All referenced CSS classes are defined in the new stylesheet, and the structural hierarchy is preserved.Also applies to: 167-167, 193-196, 227-227, 237-242, 264-264, 311-311, 350-350, 357-357
src/main/resources/META-INF/frontend/styles/fc-recurrent-schedule-field.css (1)
18-52: Verify CSS nesting support in Vaadin.The CSS uses native nesting syntax (e.g.,
.fc-rsf-linespan { div { ... } p { ... } }), which requires either modern browser support for CSS Nesting or a CSS preprocessor. Vaadin's default CSS handling may not process nested selectors.Verify whether Vaadin supports CSS nesting by checking if the nested rules render correctly:
1 participant
Close #7
Summary by CodeRabbit
New Features
Style
Demos