Skip to content
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

feat(storybook): updated 2 component stories to v7 #11362

Conversation

m4olivei
Copy link
Contributor

@m4olivei m4olivei commented Jan 8, 2024

Related Ticket(s)

Closes #11356

Description

Adjustments to search and select component stories to be compatible with Storybook v7.

Changelog

Changed

  • Updated search Storybook stories for Storybook v7
  • Updated select Storybook stories for Storybook v7

@m4olivei m4olivei requested a review from a team as a code owner January 8, 2024 21:39
@m4olivei m4olivei requested review from emyarod and IgnacioBecerra and removed request for a team January 8, 2024 21:39
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Jan 8, 2024

@m4olivei m4olivei marked this pull request as draft January 8, 2024 22:04
@m4olivei
Copy link
Contributor Author

m4olivei commented Jan 8, 2024

Hum, the <cds-select> element isn't rendering here. Error in console:

Uncaught (in promise) TypeError: Cannot read properties of null (reading 'selectedIndex')
    at get selectedIndex (select-qPaB998O.js:19:496)
    at d.get [as selectedIndex] (lit-element-XNu4h86T.js:9:1284)
    at d.performUpdate (lit-element-XNu4h86T.js:9:5121)
    at d.scheduleUpdate (lit-element-XNu4h86T.js:9:4791)
    at d._$EP (lit-element-XNu4h86T.js:9:4699)

I don't get this locally. Will have to do some debugging.

Back to draft.

@m4olivei
Copy link
Contributor Author

m4olivei commented Jan 9, 2024

I trimmed back the import statements for the story in c8d76c6. That triggered the same error, which I was able to debug. Lit's lifecycle code was calling into the selectedIndex getter early, before this._selectNode (decorated with @query) had been populated. Fixed in 820b111.

@m4olivei m4olivei marked this pull request as ready for review January 9, 2024 18:10
Copy link
Contributor

@IgnacioBecerra IgnacioBecerra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@kennylam kennylam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like select has readOnly twice. Otherwise looks good!

image

@m4olivei
Copy link
Contributor Author

Looks like select has readOnly twice. Otherwise looks good!

Nice catch. Fixed!

@kennylam kennylam merged commit b9743ef into carbon-design-system:feat/cwc-storybook-7-vite Jan 10, 2024
7 of 14 checks passed
kennylam added a commit to kennylam/carbon-for-ibm-dotcom that referenced this pull request Jun 11, 2024
…tem#11362)

* chore(search): update search stories to sb v7

* chore(select): update select stories to sb v7

* chore(select): trim back select imports

* fix(select): use optional chaining on a @query property inside getter

* chore(select): fix case on readOnly

---------

Co-authored-by: kennylam <909118+kennylam@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants