-
Notifications
You must be signed in to change notification settings - Fork 74
Tab: Adding id as an "optional" params #2872
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?
Tab: Adding id as an "optional" params #2872
Conversation
✅ Deploy Preview for moduswebcomponents ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
@roaldjap Can we use the index instead of generating a new ID? It’s challenging to identify the selected tab since the ID is created within the component when I want to show different tab content. Also it would be great if you can update the interface and storybook documentation.
…/iterating suffix tab-label class in DOM to ensure active state
9e05b5d
to
ab376ed
Compare
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.
-
File:
stencil-workspace/src/components/modus-tabs/modus-tabs.tsx
(line 88-93) The new code adds automatic ID generation for tabs when no ID is provided, but the kebabCase function doesn't handle null or undefined inputs safely. If tab.label is null or undefined, the function will throw an error when calling .match() on it. Consider adding a null check before calling kebabCase. -
File:
stencil-workspace/src/utils/utils.ts
(line 20-25) The kebabCase implementation assumes the input string will match the regex pattern and return an array, but it doesn't handle edge cases where the match returns null (empty strings, strings with only special characters). The .join() call would throw an error in such cases. -
File:
stencil-workspace/src/components/modus-tabs/modus-tabs.e2e.ts
(line 65) The test case uses a mixed approach for tab IDs (one tab without ID, one with numeric ID 0), but doesn't verify that both tabs render correctly. Consider expanding test coverage to validate both tabs. -
File:
stencil-workspace/src/utils/utils.spec.ts
(line 23-27) The unit test for kebabCase only tests a single case with whitespace. Add more test cases for different inputs like camelCase, PascalCase, and edge cases like empty strings or strings with special characters.
Description
I added a functionality where
modusTabs.tabs
does not need to require to provide anid
per tab instance - as label would be enough to generate a id for this to ensureactive
state are clicking even id were not provided.Coincidentally, This also resolves an issue that relates to: #2197
References #
#2197
Type of change
How Has This Been Tested?
To reproduce this - initially I created a tab instance like this:
I noticed tabs were not changing its active state - even if
active: true
is set on first tab instance.Checklist