-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat: add rel
property to relevant elements to override default behavior
#814
feat: add rel
property to relevant elements to override default behavior
#814
Conversation
Hi there @AndyLongShout, thank you for this contribution! 👍 While we wait for the team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
Quality Gate failedFailed conditions |
There where a few errors when running test that where from original code should I try and fix those or do want a different issue raised for that? Sonar Cloud scan seems to be new but do you want me to update anything to fix these errors? |
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.
@AndyLongShout I think this looks very nice.
Don't worry about SonarCloud for now.
If you find other bugs, feel free to fix them in a new PR.
This tests out alright, so I'll be able to merge it in. Thank you :)
rel
property to relevant elements to override default behavior
…avior (#814) * 721 property added for rel to allow * 721 fix tab close tag
Description
Updates to the following uui components to add the option for a rel property to allow it to be set by the component rather than just relying on the target="_blank"
uui-button
uui-button-inline - not sure if this is being maintained as the stories and test don't map 100% to the compoment
uui-card
uui-card-block-type
uui-card-content-node
uui-card-media
uui-card-user
uui-menu-item
uui-ref-node
uui-tabs
To ensure it doesn't effect existing links with a target _blank I have left the original logic in to set it to what was previously there but allow it to be overidden by rel prop.
Types of changes
Motivation and context
Proposed fix for issue #721
Gived the ability to define the rel on a link
Picked this up as my first contribution to Umbraco
How to test?
Within the following components there is a new prop for rel if you set the rel the value will be set on the anchor tag.
uui-button
uui-button-inline - not sure if this is being maintained as the stories and test don't map 100% to the compoment
uui-card
uui-card-block-type
uui-card-content-node
uui-card-media
uui-card-user
uui-menu-item
uui-ref-node
uui-tabs
Screenshots (if appropriate)
Checklist