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

Vue component considers 0 (for rotation) and md (for size) despite them being legitimate values #493

Open
mattburlage opened this issue Mar 21, 2024 · 6 comments

Comments

@mattburlage
Copy link

mattburlage commented Mar 21, 2024

The vue component throws a warning when rotation is 0 or size is md, making it difficult to determine those values programmatically. Having those as defaults is reasonable, but the component should also accept those values explicitly.

<FontAwesomeIcon /> shouldn't consider 0 (for rotation) or 'md' (for size) as invalid prop values.

For example:

<FontAwesomeIcon
   :icon="ICON"
   :rotation="ROTATION"
   :size="SIZE"
/>
@Emmo00
Copy link

Emmo00 commented Apr 4, 2024

This rotation props controls the fa-rotate- class, and according to the (docs)[https://docs.fontawesome.com/web/style/rotate], the only angles available are: 90, 180 and 270.

a temporary fix (to remove the warning) might be to just add 0 to the list of expected values by the vue component, this would add a fa-rotate-0 class to the icon - which does not have a style implemented.

Another way might be to implement (Custom fix)[https://docs.fontawesome.com/web/style/rotate#custom-rotation] for the rotation prop... this might be a more involved patch.

I'd be happy to write a patch

@mattburlage
Copy link
Author

mattburlage commented Apr 4, 2024

Would it be possible for the component to just not add fa-rotate- if it detects 0? Likewise, not apply a size if md is detected for that attribute? Maybe here?

export function classList (props) {

Something like:

export function classList (props) {
  const size = props.size !== 'md' ? props.size : null;
  const rotation = props.rotation !== 0 ? props.rotation : null;

  let classes = {
    'fa-spin': props.spin,
    'fa-pulse': props.pulse,
    'fa-fw': props.fixedWidth,
    'fa-border': props.border,
    'fa-li': props.listItem,
    'fa-inverse': props.inverse,
    'fa-flip': props.flip === true,
    'fa-flip-horizontal': props.flip === 'horizontal' || props.flip === 'both',
    'fa-flip-vertical': props.flip === 'vertical' || props.flip === 'both',
    [`fa-${size}`]: size !== null,
    [`fa-rotate-${rotation}`]: rotation !== null,
    [`fa-pull-${props.pull}`]: props.pull !== null,
    'fa-swap-opacity': props.swapOpacity,
    'fa-bounce': props.bounce,
    'fa-shake': props.shake,
    'fa-beat': props.beat,
    'fa-fade': props.fade,
    'fa-beat-fade': props.beatFade,
    'fa-flash': props.flash,
    'fa-spin-pulse': props.spinPulse,
    'fa-spin-reverse': props.spinReverse
  }

  return Object.keys(classes)
    .map(key => classes[key] ? key : null)
    .filter(key => key)
}

Since the object definition already handles if those attributes are null, this should accomplish the goal without adding extra classes. Thoughts?

@mattburlage
Copy link
Author

I suppose you could also just handle this in the object key-value assignments and avoid the extra variables.

    [`fa-${props.size}`]: ['md', null].includes(props.size),
    [`fa-rotate-${props.rotation}`]: [0, null].includes(props.rotation),

@Emmo00
Copy link

Emmo00 commented Apr 4, 2024

You're right

@Emmo00
Copy link

Emmo00 commented Apr 4, 2024

    [`fa-${props.size}`]: ['md', null].includes(props.size),
    [`fa-rotate-${props.rotation}`]: [0, null].includes(props.rotation),

Yes, this looks nice, but Did you mean to negate this like

[`fa-rotate-${props.rotation}`]: ![0, null].includes(props.rotation)

?

@mattburlage
Copy link
Author

mattburlage commented Apr 4, 2024 via email

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

No branches or pull requests

3 participants