-
Notifications
You must be signed in to change notification settings - Fork 2
feat(LIB-1912): FzTab 1.0.0 #283
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: main
Are you sure you want to change the base?
Conversation
| v-if="tab.badgeContent != null" | ||
| :color="selectedTab === tab.title ? 'blue' : 'black'" | ||
| :size="size" | ||
| :color=" |
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.
Nested ternary used for the :color attribute (selectedTab === tab.title ? props.tone === 'alert' ? 'error' : 'blue' : 'black'). Extract into a small helper or computed property to improve clarity.
Details
✨ AI Reasoning
A nested ternary was introduced in the template attribute that chooses the badge color based on selectedTab and tone. Nested ternaries combine multiple decision branches on one line, making it harder to read and maintain. This harms readability and increases the chance of mistakes when modifying logic.
🔧 How do I fix it?
Break long lines to enhance clarity. Aim for a maximum of 80-120 characters per line, depending on the context and coding standards.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
cbe0be6 to
61b400f
Compare
- Updated tests to reflect the deprecation of the size prop, applying default size classes instead. - Adjusted emitted change event assertions to include both title and button element in the payload. - Enhanced snapshot tests to match updated component structure and styles.
| size: 'md' | ||
| } | ||
| } | ||
|
|
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.
Storybook tests expect single-argument emit but receive two
Medium Severity
The change event emit signature was changed from emit("change", selectedTab.value) to emit("change", selectedTab.value, selectedTabElement) in FzTabs.vue, but existing Storybook interaction tests in the Disabled, KeyboardNavigation, and UserInteraction stories still use toHaveBeenCalledWith('tab title') expecting exactly one argument. These assertions will fail because toHaveBeenCalledWith requires an exact match of all arguments, and the emit now passes two.
Additional Locations (2)
| return "picker"; | ||
| return props.tabStyle; | ||
| }); |
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.
effectiveTabStyle priority logic contradicts stated design and effectiveSize
Medium Severity
The effectiveTabStyle computed gives horizontalOverflow: false higher priority than the tabStyle prop. When both are set (e.g., { tabStyle: 'scroll', horizontalOverflow: false }), the deprecated horizontalOverflow wins and returns 'picker'. This contradicts the comment stating tabStyle should have priority, and is inconsistent with effectiveSize which correctly prioritizes the new environment prop over deprecated size.
…dling - Added '@fiscozen/action' and '@fiscozen/icons' as dependencies in the tab package. - Updated FzTabs to utilize FzAction components for tab selection in picker mode. - Improved tone handling for alert and neutral styles in tab buttons. - Refactored tests to accommodate new structure and ensure proper rendering and interaction. - Updated snapshots to reflect changes in component structure and styles.
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| v-if="selectedTabProps" | ||
| :tab="selectedTabProps" | ||
| :environment="environment" | ||
| :tone="props.tone" |
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.
Picker mode doesn't use selected tab's tone styling
Medium Severity
In picker mode, the opener button uses props.tone from FzTabPicker, but FzTabs never passes the tone prop when rendering FzTabPicker. Meanwhile, in scroll mode, each FzTabButton correctly receives :tone="tab.tone". This causes inconsistent behavior: a tab with tone="alert" displays alert styling in scroll mode but neutral styling in picker mode, because props.tone is undefined and defaults to neutral.
Additional Locations (1)
| }, | ||
| }; | ||
|
|
||
| export const mapUnselectedTabToClasses = { |
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.
Size-to-environment migration mapping contradicts documentation
Medium Severity
The mapSizeToEnvironment maps md to "frontoffice", but the documentation explicitly states both size="sm" and size="md" should map to environment="backoffice". Users migrating from the deprecated size="md" prop will get frontoffice (h-48) instead of the documented backoffice (h-40), causing unexpected visual changes.
Jira: LIB-1912
Note
Major revamp of
@fiscozen/tabfor v1.0.0 with new API, styling, and docs, plus extensive tests.environment,tabStyle,tone, andisDebug; overflow now controlled viatabStyle(scrollorpicker)size,horizontalOverflow, andvertical(warnings viadebugWarn); maps old props for backward compatibilitychangenow emits[title, selectedTabElement]FzTabButton/FzTabPickerto use environment-based sizing and tone-aware classes; adds@fiscozen/action/@fiscozen/iconsdepsFzTabs.mdx) and reorganized stories; large unit and snapshot test suite added/updatedWritten by Cursor Bugbot for commit a895be8. This will update automatically on new commits. Configure here.
Summary by Aikido
🚀 New Features
⚡ Enhancements
🔧 Refactors
📚 Documentation
More info