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: Port CvButtonSkeleton to Vue3 #1470

Merged
merged 4 commits into from
Jul 15, 2023
Merged

feat: Port CvButtonSkeleton to Vue3 #1470

merged 4 commits into from
Jul 15, 2023

Conversation

OlkaB
Copy link
Contributor

@OlkaB OlkaB commented Jul 5, 2023

Contributes to #1469

## What did you do?
Port CvButtonSkeleton to Vue3

## Why did you do it?
Hope this will help to finish transition before Vue2 EOL

How have you tested it?

please see CvButtonSkeleton.spec.js + CvButtonSkeleton.stories.js
compared behaviours with Vue2 storybook

Were docs updated if needed?

  • Yes

@OlkaB OlkaB changed the title Port CvButtonSkeleton to Vue3 feat: Port CvButtonSkeleton to Vue3 Jul 5, 2023
@davidnixon davidnixon self-assigned this Jul 11, 2023
Copy link
Collaborator

@davidnixon davidnixon left a comment

Choose a reason for hiding this comment

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

Wow! Thanks for helping out here! Almost perfect. the only thing I see is that CvButtonSkeleton is imported in index.js but not exported. So for me I see nothing in the story and I see this in the console log:

[Vue warn]: Failed to resolve component: cv-button-skeleton
and a blank story
image

When I add skeleton to the export like:

export {
  CvButton,
  CvButtonConsts,
  CvButtonSet,
  CvIconButton,
  CvButtonSkeleton,
};

All works great!
image

Extra

For bonus points, since you are in that code anyway, ... could I ask for a separate fix unrelated to skeleton?
The main button stories are in src/components/CvButton/CvButton.stories.js but some of them have an issue. The button is blank like this:
image

The issue is that somehow we have

'slotArgs.default': 'Field size',

instead of

default: 'Field size',

Could you update the button stories to fix that too?

@OlkaB
Copy link
Contributor Author

OlkaB commented Jul 12, 2023

Thanks @davidnixon . Export added (sorry missed that).
The requested fix for buttons stories will be done in separate PR.

btw. As I'm not using github for PRs on a daily basis let me please know if I should do sth here (differently?)

import CvButtonSkeleton from '../CvButtonSkeleton.vue';

describe('CvButtonSkeleton', () => {
test.each([[undefined], [''], ['default']])(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have not used that test.each pattern before. Very cool!

@davidnixon davidnixon merged commit 700325b into carbon-design-system:vNext Jul 15, 2023
2 checks passed
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

2 participants