Vue2 add bounce, shake, and some tests #364
Conversation
| @@ -37,7 +37,7 @@ if(coreHasFeature(ICON_ALIASES)) { | |||
| const vm = mountFromProps({ icon: ['fas', 'close'] }) | |||
|
|
|||
| expect(vm.$el.tagName).toBe('svg') | |||
| expect(vm.$el.classList.contains('fa-close')).toBeTruthy() | |||
| expect(vm.$el.classList.contains('fa-xmark')).toBeTruthy() | |||
There was a problem hiding this comment.
fix failing test here. xmark is the icon name, close is the alias.
jasonlundien
force-pushed
the
bounce-shake-test
branch
from
de7cc99 to
0d75f03
Compare
There was a problem hiding this comment.
Everything looks great! I think @robmadole should review before we merge.
jasonlundien
changed the title
DEVELOPMENT.md
Outdated
| @@ -16,8 +16,8 @@ test | Execute unit tests | |||
| 1. Update `package.json` and change `version` | |||
| 1. Update `README.md` and `package.json`; adding any contributors | |||
| 1. Update the `CHANGELOG.md` | |||
| 1. `npm publish` | |||
| 1. `npm publish --registry https://npm.fontawesome.com` | |||
| 1. `npm publish --tag 2x` | |||
There was a problem hiding this comment.
@jasonlundien I've looked at a few projects and there doesn't seem to be a solid convention for how you tag major versions.
So let's do it like this:
- If it's compatible with Vue 2:
latest-2 - For Vue 3:
latest-3
package.json
Outdated
| "test.latest": "npm --no-save install @fortawesome/fontawesome-svg-core@latest @fortawesome/free-solid-svg-icons@latest && jest --silent", | ||
| "test.next": "npm --no-save install @fortawesome/fontawesome-svg-core@next @fortawesome/free-solid-svg-icons@next && jest --silent", | ||
| "test.next.proregistry": "npm --userconfig .npmrc.proregistry --registry https://npm.fontawesome.com install --no-save @fortawesome/fontawesome-svg-core@next @fortawesome/free-solid-svg-icons@next && jest --silent", | ||
| "test": "npm --no-save install @fortawesome/fontawesome-svg-core @fortawesome/free-solid-svg-icons && jest --silent", |
There was a problem hiding this comment.
@jasonlundien @ej2 can we update the tests so that it works like our React component?
https://github.com/FortAwesome/react-fontawesome/blob/master/package.json#L38-L40
The reason for both of them is so that we can test with the FA 5 and 6 versions.
| type: String, | ||
| default: null, | ||
| validator: (value) => ['horizontal', 'vertical', 'both'].indexOf(value) > -1 | ||
| type: [Boolean, String], |
There was a problem hiding this comment.
I see what you did there...that's clever. Nice.
This seems rather tricky now. Have we documented how this works?
There was a problem hiding this comment.
What is interesting about flip is that we use flip as a standalone animation that actually continuously flips the icon over and over. Ex: <i class="fa-solid fa-compact-disc fa-flip"></i>
And we also use flip to rotate icons (not an animation it just rotates the icon, not continuously). Ex. <i class="fa-solid fa-snowboarding fa-flip-vertical"></i>
We do have both documented:
Rotate: https://fontawesome.com/docs/web/style/rotate#rotate-classes
Animation: https://fontawesome.com/docs/web/style/animate#flip
The flip animation was NOT working with our Vue 2 or Vue 3 packages... this fixes it.
| test('horizontal', () => { | ||
| const vm = mountFromProps({ icon: faCoffee, flip: "horizontal" }) | ||
|
|
||
| expect(vm.$el.classList.contains('fa-flip-horizontal')).toBeTruthy() | ||
| expect(vm.$el.classList.contains('fa-flip-vertical')).toBeFalsy() |
There was a problem hiding this comment.
I think if you add expect(vm.$el.classList.contains('fa-flip')).toBeFalsy() you will get a surprising test failure...I will comment down below as to why I think this.
src/utils.js
Outdated
| @@ -15,12 +15,16 @@ export function classList (props) { | |||
| 'fa-border': props.border, | |||
| 'fa-li': props.listItem, | |||
| 'fa-inverse': props.inverse, | |||
| 'fa-flip': props.flip, | |||
There was a problem hiding this comment.
This probably needs to be
| 'fa-flip': props.flip, | |
| 'fa-flip': props.flip === true, |
Otherwise since a string is truthy I think anytime the value is horizontal or vertical or both you'll get the fa-flip class applied.
There was a problem hiding this comment.
You were/are right about the failing test @robmadole without having this... I actually saw my icons continuously rotating and ignored it for whatever reason. This addition, however, does prevent the icons from flipping yet allowing them to have been "rotated" as desired.
PR adds bounce, shake, beat-fade and tests in vue-fontawesome 2.x.
Also updates the deps (thank you @ej2)!