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

Convert Add group link to button #38202

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented May 11, 2023

Summary

Convert "Add group" action to button instead of link since it does not have a "route" but just an on-page action.

Checklist

Before

before

After

after

@nfebe nfebe force-pushed the fix/37135/addgroup-link-to-button branch from 1c77203 to 6ae4495 Compare May 11, 2023 14:13
@nfebe nfebe force-pushed the fix/37135/addgroup-link-to-button branch 3 times, most recently from 473f54c to b862829 Compare May 13, 2023 14:13
@nfebe nfebe changed the title WIP: Convert Add group link to button Convert Add group link to button May 13, 2023
@nfebe nfebe marked this pull request as ready for review May 13, 2023 14:16
@nfebe
Copy link
Contributor Author

nfebe commented May 13, 2023

I believe this would have to be completed in nextcloud/nextcoud-vue but that this is a step towards the intended outcome.

@nfebe nfebe requested review from skjnldsv, artonge and come-nc May 13, 2023 14:18
The "Add group" peforms an on-page action and does not route or link to
anywhere else hence, not semantically a link but a button.

This commit implements the ehancement described at the respository level.

A change would be required in [@nextcloud/nextcloud-vue](nextcloud-libraries/nextcloud-vue#4108)
since the `NcAppNavigationNewItem` is an out-of-repo dependency.

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe force-pushed the fix/37135/addgroup-link-to-button branch from b862829 to 1ee8fba Compare May 14, 2023 00:01
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add before/after screenshots if this changes anything visually?

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

@nfebe nfebe merged commit e7ba255 into master May 16, 2023
@nfebe nfebe deleted the fix/37135/addgroup-link-to-button branch May 16, 2023 00:23
@blizzz blizzz mentioned this pull request May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants