-
Notifications
You must be signed in to change notification settings - Fork 133
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
Vue 3x - Update for titleId and maskId prop #455
Conversation
src/components/FontAwesomeIcon.js
Outdated
}, | ||
titleId: { | ||
type: String, | ||
default: null, |
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.
added titleId
src/components/FontAwesomeIcon.js
Outdated
}, | ||
maskId: { | ||
type: String, | ||
default: null, |
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.
added maskId
Prettier has taken over these files as well @robmadole. I left comments where I changed the code (6 total).
|
a22cfc4
to
3072263
Compare
@jasonlundien the changes look good. But the stuff prettier did is a little annoying. Were you surprised by them? I almost wonder if something is misconfigured in VSCode. |
@robmadole ---
I went back and adjusted my prettier settings to reflect those of Font Awesome. The prettier changes are now fewer than they were but you will still see some the changes --- for consistency. The exception is the I am fine with these prettier changes and being consistent going forward if you are ?! |
d458f1f
to
251746c
Compare
7626f9a
to
444dbfa
Compare
91d7b79
to
56462a1
Compare
const wrapper = mountFromProps({ icon: faCoffee, size: size }) | ||
;['2xs', 'xs', 'sm', 'lg', 'xl', '2xl', '1x', '2x', '3x', '4x', '5x', '6x', '7x', '8x', '9x', '10x'].forEach( | ||
(size) => { | ||
const wrapper = mountFromProps({ icon: faCoffee, size: size }) |
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.
prettier
} | ||
}, | ||
{ immediate: true } | ||
) |
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.
prettier
|
||
const vnode = computed(() => renderedIcon.value ? convert(renderedIcon.value.abstract[0], {}, attrs) : null) | ||
const vnode = computed(() => (renderedIcon.value ? convert(renderedIcon.value.abstract[0], {}, attrs) : null)) |
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.
prettier
title: props.title, | ||
titleId: props.titleId, | ||
maskId: props.maskId | ||
}) |
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.
prettier + added titleId: props.titleId,
and maskId: props.maskId
? faParse.transform(props.transform) | ||
: props.transform | ||
)) | ||
const transform = computed(() => objectWithKey('transform', typeof props.transform === 'string' ? faParse.transform(props.transform) : props.transform)) |
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.
prettier
}, | ||
|
||
setup (props, { attrs }) { | ||
setup(props, { attrs }) { |
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.
prettier
@@ -133,35 +141,38 @@ export default defineComponent({ | |||
spinReverse: { | |||
type: Boolean, | |||
default: false | |||
}, | |||
} |
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.
prettier
@@ -4,7 +4,7 @@ import convert from '../converter' | |||
import log from '../logger' | |||
import { objectWithKey, classList } from '../utils' | |||
|
|||
function normalizeIconArgs (icon) { | |||
function normalizeIconArgs(icon) { |
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.
prettier
bab2321
to
b5e467a
Compare
3b3741b
to
3c6538d
Compare
3c6538d
to
30929b9
Compare
c95cac7
to
15852d3
Compare
This PR addresses this bug report: title property breaks snapshots#181
Currently we cannot add and set a title property without a random generated string being attached to the
aria-labelledby
and thetitle id
.This PR will allow us to set a
title
andtitleId
so that there would NOT be a random generated string if set, and snapshot tests will now pass.@robmadole --- can you review code for accuracy?