feat(core)!: Extend colors with base color for text based components#143
Merged
michaelcozzolino merged 9 commits into3.xfrom Mar 13, 2026
Merged
Conversation
…ng, and Link components
…g, Heading, and Link
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis pull request refactors the text color system. It adds a 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Deploying flyonui-vue with
|
| Latest commit: |
b3a7108
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34ca640e.flyonui-vue.pages.dev |
| Branch Preview URL: | https://feat-extend-colors-for-text.flyonui-vue.pages.dev |
Deploying flyonui-vue-v3 with
|
| Latest commit: |
b3a7108
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f0e6703a.flyonui-vue-v3.pages.dev |
| Branch Preview URL: | https://feat-extend-colors-for-text.flyonui-vue-v3.pages.dev |
…changes for text-based components
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/docs/Next/Content/Divider/DashedDivider.vue (1)
60-64: Optional: add a horizontalBasedashed sample for parity.Line 60 onward starts the horizontal set, but unlike the vertical set, it doesn’t include a
color="base"example. Adding one would keep both sections symmetric.Suggested doc-only patch
<FoDivider preset="dash" orientation="horizontal" > Default </FoDivider> +<FoDivider color="base" + preset="dash" + orientation="horizontal" +> + Base +</FoDivider> + <FoDivider color="neutral" preset="dash" orientation="horizontal" > Neutral🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/Next/Content/Divider/DashedDivider.vue` around lines 60 - 64, Add a horizontal "Base" dashed example to match the vertical section by adding another FoDivider instance in the horizontal group using preset="dash" and orientation="horizontal" with color="base" (i.e., a second FoDivider alongside the existing Default example). Ensure the new FoDivider uses the same structure and props as the Default horizontal sample but adds color="base" so both sections are symmetric.packages/core/tests/Lib/UseColor/Internal/Lib/UseTextColor.spec.ts (1)
14-18: Test coverage gap: undefined color path is no longer tested.The new test for
'base'color is correct. However, per the AI summary, a test for the undefined color case was removed. The implementation inUseTextColor.tsstill acceptsTextColor | undefinedand passes undefined touseColor. Components likeFoLinkmay calluseTextColorwithprops.colorbeing undefined.Consider adding a test case to verify the behavior when color is undefined:
it('returns a default color class if the color is undefined', () => { const config = ref<FlyonUIVueAppDefaultConfig>({ ...flyonUIVueAppDefaultConfig }); expect(useTextColor(config, 'FoLink', () => undefined).value).toBe(/* expected default */); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/Lib/UseColor/Internal/Lib/UseTextColor.spec.ts` around lines 14 - 18, Add a test that covers the undefined color path for useTextColor: call useTextColor with config (ref of FlyonUIVueAppDefaultConfig from flyonUIVueAppDefaultConfig), the component name 'FoLink', and a color provider that returns undefined (e.g., () => undefined), then assert the returned .value equals the expected default text class (the same fallback class used by useTextColor/useColor, e.g., 'text-base-content'); this ensures useTextColor and useColor handle props.color === undefined correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/Lib/UseColor/Internal/Lib/UseTextColor.ts`:
- Around line 1-4: Reorder the imports to satisfy perfectionist/sort-imports:
group and alphabetize by module and separate type-only imports from value
imports. Specifically, keep the type-only Vue imports together (ComputedRef,
MaybeRefOrGetter, Ref) and the value Vue imports together (computed, toValue),
then import types from '@/Lib' (FlyonUIVueAppDefaultConfig, TextColor,
TextColorableComponentName), and finally the value import useColor from
'@/Lib/UseColor/Internal'; update the four import statements to that order so
ESLint stops flagging them.
- Around line 12-19: The computed in useTextColor currently returns
useColor(config, componentName, c).value which can be undefined, violating the
declared ComputedRef<string>; update the computed to guard the result (e.g.
capture const value = useColor(config, componentName, c).value and return value
?? 'text-base-content' or another appropriate default) so the computed always
yields a string; keep references to toValue(color), c, config and componentName
intact.
In `@packages/docs/UpgradeGuide.md`:
- Around line 40-44: Fix the awkward phrasing in the sentence describing the
color prop for FoDivider / FoHeading / FoLink / FoLoading: change "if you want
now this color you have to pass the `base` value to the `color` prop." to "if
you now want this color, you have to pass the `base` value to the `color` prop."
so the sentence reads smoothly and includes the comma before the clause.
---
Nitpick comments:
In `@packages/core/tests/Lib/UseColor/Internal/Lib/UseTextColor.spec.ts`:
- Around line 14-18: Add a test that covers the undefined color path for
useTextColor: call useTextColor with config (ref of FlyonUIVueAppDefaultConfig
from flyonUIVueAppDefaultConfig), the component name 'FoLink', and a color
provider that returns undefined (e.g., () => undefined), then assert the
returned .value equals the expected default text class (the same fallback class
used by useTextColor/useColor, e.g., 'text-base-content'); this ensures
useTextColor and useColor handle props.color === undefined correctly.
In `@packages/docs/Next/Content/Divider/DashedDivider.vue`:
- Around line 60-64: Add a horizontal "Base" dashed example to match the
vertical section by adding another FoDivider instance in the horizontal group
using preset="dash" and orientation="horizontal" with color="base" (i.e., a
second FoDivider alongside the existing Default example). Ensure the new
FoDivider uses the same structure and props as the Default horizontal sample but
adds color="base" so both sections are symmetric.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92c9c7c1-d649-4454-a01d-920a6c352f91
⛔ Files ignored due to path filters (6)
packages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-divider-dashed-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-divider-dotted-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-divider-solid-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-divider-with-custom-thickness-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-heading-color-chromium-linux.pngis excluded by!**/*.pngpackages/docs/tests/EndToEnd/Docs.browser.spec.ts-snapshots/content-link-color-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (26)
packages/core/src/Lib/UseColor/Internal/Lib/UseTextColor.tspackages/core/src/Lib/UseColor/Types/Color.tspackages/core/src/UI/Components/Loading/Types/Loading.tspackages/core/src/UI/Content/Divider/Types/Divider.tspackages/core/src/UI/Content/Heading/Types/Heading.tspackages/core/src/UI/Content/Heading/UI/FoHeading.vuepackages/core/src/UI/Content/Link/Types/Link.tspackages/core/tests/Lib/UseColor/Internal/Lib/UseTextColor.spec.tspackages/docs/Components/Loading/LoadingAnimation.vuepackages/docs/Components/Loading/LoadingColor.vuepackages/docs/Content/Heading/HeadingColor.vuepackages/docs/Content/Heading/HeadingLevel.vuepackages/docs/Content/Link/AnimatedUnderlineLink.vuepackages/docs/Content/Link/DefaultLink.vuepackages/docs/Content/Link/LinkColor.vuepackages/docs/Content/Link/UnderlineOnHoverLink.vuepackages/docs/Next/Content/Divider/DashedDivider.vuepackages/docs/Next/Content/Divider/DividerContentAlignment.vuepackages/docs/Next/Content/Divider/DividerWithCustomThickness.vuepackages/docs/Next/Content/Divider/DottedDivider.vuepackages/docs/Next/Content/Divider/HorizontalDivider.vuepackages/docs/Next/Content/Divider/HorizontallyResponsiveDivider.vuepackages/docs/Next/Content/Divider/SolidDivider.vuepackages/docs/Next/Content/Divider/VerticalDivider.vuepackages/docs/Next/Content/Divider/VerticallyResponsiveDivider.vuepackages/docs/UpgradeGuide.md
michaelcozzolino
deleted the
feat/extend-colors-for-text-based-components
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests