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

Add button role for NcButton with href #3532

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Add button role for NcButton with href #3532

merged 2 commits into from
Dec 5, 2022

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Dec 1, 2022

Attribute role="button" added for anchor element in NcButton with href and fixed the invalid type="button" attr being added to the DOM when using NcButton as a link https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-type

router-link is rendered as a button element https://v3.router.vuejs.org/api/#tag

@Pytal Pytal added bug Something isn't working 3. to review Waiting for reviews feature: actions Related to the actions components labels Dec 1, 2022
@Pytal Pytal added this to the 7.1.1 milestone Dec 1, 2022
@Pytal Pytal self-assigned this Dec 1, 2022
@raimund-schluessler
Copy link
Contributor

@Pytal Are you sure, this is necessary? Looking at nextcloud/server#35527 I suppose this is about passing href to the NcButton. But that should work already by passing the propsData here:
https://github.com/nextcloud/nextcloud-vue/blob/4e8546981baa32b3c1ec34825fa3a7f02db182d7/src/components/NcActions/NcActions.vue#L949
At least in the docs, a single NcActionLink with href set correctly becomes a NcButton of type link with the href present.

@Pytal
Copy link
Contributor Author

Pytal commented Dec 2, 2022

This is for the attribute role="button" from this comment as it is not included in propsData @raimund-schluessler, open to ideas if another way is better?

@raimund-schluessler
Copy link
Contributor

This is for the attribute role="button" from this comment as it is not included in propsData @raimund-schluessler, open to ideas if another way is better?

If role="button" is necessary, maybe it's a good idea to add it to NcButton by default? @skjnldsv what do you think?

@Pytal
Copy link
Contributor Author

Pytal commented Dec 2, 2022

@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 2, 2022

If role="button" is necessary, maybe it's a good idea to add it to NcButton by default? @skjnldsv what do you think?

Absolutely 👍

Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal changed the title Correctly pass attrs in NcActions Add button role for NcButton with href Dec 3, 2022
@PVince81 PVince81 merged commit 7bd8617 into master Dec 5, 2022
@PVince81 PVince81 deleted the enh/action-attrs branch December 5, 2022 11:24
@skjnldsv skjnldsv modified the milestones: 7.1.1, 7.2.0 Dec 6, 2022
@skjnldsv skjnldsv mentioned this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: actions Related to the actions components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants