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: support object class binding in stubbed functional components #1476

Merged

Conversation

lmiller1990
Copy link
Member

fixes #1474

Copy link

@kierans kierans left a comment

Choose a reason for hiding this comment

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

Looks good to me! Well done @lmiller1990 👍

// see https://github.com/vuejs/vue-test-utils/issues/1474 for more context.
if (typeof dynamicClass === 'object') {
evaluatedDynamicClass = Object.keys(dynamicClass).reduce((acc, key) => {
if (dynamicClass[key] === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check if this is a truthy value or like this, straight up true?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, I just went with this to be explicit. I haven't seen people use something like :class={ 'my-class': 'some-truthy-val' }. I guess we should just copy whatever Vue does - will try it out.

}
return staticClass || dynamicClass
return staticClass || evaluatedDynamicClass
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldnt have pushing all to an array and using join(' ') at the end been a more cleaner way of doing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think both are fine - was having problems iterating over Object.keys, using of throws some warning. A few other places in the codebase do Object.keys().reduce so I just tried to follow that same style.

I like reduce but I don't mind either way.

@lmiller1990 lmiller1990 merged commit 55f7eac into dev Mar 20, 2020
@lmiller1990 lmiller1990 deleted the feature/1474_support_dynamic_class_in_functional_component branch March 20, 2020 09:52
@dobromir-hristov
Copy link
Contributor

gg

@kierans
Copy link

kierans commented Mar 20, 2020

What's the release process please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic classes not added correctly to wrapper classes when using shallowMount
4 participants