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

Update on-background attribute for ease of use in Vue 3 #307

Closed
michael-iden opened this issue Feb 9, 2022 · 8 comments · Fixed by #578
Closed

Update on-background attribute for ease of use in Vue 3 #307

michael-iden opened this issue Feb 9, 2022 · 8 comments · Fixed by #578

Comments

@michael-iden
Copy link
Contributor

michael-iden commented Feb 9, 2022

Expected behavior
When on-background attribute is applied to the button, the color scheme of the button is properly reflected

Actual behavior
on-background attribute is removed from the DOM, likely by Vue 3, possibly because it starts with on* and Vue 3 interprets that to be an event listener.

Steps to reproduce the issue

  1. Create pharos button with on-background set on the element
  2. Load up the app and see that the on-background is removed from the DOM
  3. Button does not appear as we would expect

Screenshots or code
I tried making a codepen/codesandbox/stackblitz but each environment was having various issues. I'll keep trying to get a working example.

Screen.Recording.2022-02-09.at.12.23.57.PM.mov

Pharos version
11.2.1

Your environment

  • OS: macOS
  • Browser: Chrome, Safari, Firefox
  • Version: 98, 15.3, 92.0

Additional information
I couldn't find anything in the Vue3 docs where this was happening but the button works perfectly well in the same code on a Vue2 app.

@Niznikr
Copy link
Collaborator

Niznikr commented Feb 10, 2022

@michael-iden To avoid a breaking change, you can probably use <pharos-button .onBackground="true">hi</pharos-button> instead to set the attribute via the prop. Some reading here on that.

@michael-iden
Copy link
Contributor Author

@Niznikr It works! Thanks for the great suggestion!
image

@daneah daneah added this to the Pharos v13 milestone Feb 24, 2022
@daneah daneah added feature request and removed bug labels Mar 6, 2022
@daneah daneah changed the title Button: on-background being removed by Vue3 Button: Update on-background attribute for ease of use in Vue 3 Mar 6, 2022
@chrisjbrown
Copy link
Contributor

@michael-iden is a fix still needed for this?

@michael-iden
Copy link
Contributor Author

@chrisjbrown I think we still want to fix this and include #308 in the next set of breaking changes. Looks like Dane already tagged issue/PR with the v13 milestone and I can make sure it gets included

@daneah daneah changed the title Button: Update on-background attribute for ease of use in Vue 3 Update on-background attribute for ease of use in Vue 3 Feb 22, 2023
@Ivanok
Copy link

Ivanok commented May 15, 2023

FWIW-- Using Nuxt 3 I couldn't get it working with .onBackground but instead with this: :on-background.attr

@mtorres3
Copy link
Contributor

@michael-iden is this issue still reproducible or are we able to close it now? I'm not too familiar with Vue to test it locally 😅

@michael-iden
Copy link
Contributor Author

michael-iden commented Jan 19, 2024

@mtorres3 I think this is still technically an issue until pharos 14 is released. Its handled by the PR you merged here #578 but i don't know if we are closing issues as we merge the relevant PRs or close issues once the feature is publicly available in the main release channel.

@daneah did you have any preference on how we manage issues where the work its done and we are in a holding pattern for the release?

@daneah
Copy link
Member

daneah commented Jan 19, 2024

@michael-iden I much prefer to close issues as a fix for them is merged, regardless of whether that fix has yet been released—it becomes difficult to do bookkeeping otherwise.

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

Successfully merging a pull request may close this issue.

6 participants