Add rel attribute to component wrapper #4551
Conversation
andysellick
force-pushed
the
cw-add-rel
branch
from
60555a6 to
d6b974d
Compare
| helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(rel: "nofollow") | ||
| helper.set_rel("noreferrer") | ||
| expect(helper.all_attributes[:rel]).to eql("noreferrer") | ||
| end |
There was a problem hiding this comment.
Non-blocking comment - Just wondering about the semantics of set_rel/add_rel. I assume best practice would be to ensure set_rel is only used when paired with a boolean flag (e.g. helper.set_rel() if something == true? Otherwise, if we freely used set_rel on a component it would always override what the developer has passed through as rel in a render statement. And do you think there would ever be a situation where the developer needs to distinguish whether they want a rel value they've passed through to be an add_rel or a set_rel? How would we handle that?
There was a problem hiding this comment.
I can see two use cases for the set_(thing) method (and you're right, I'll look at expanding that for the other attributes where appropriate) - one is the one you said, when paired with a boolean flag, but the other is for a component where we don't want something to be overridden, so it should always be set to what is in the template no matter what is passed.
I'll have a think about the best way of documenting this, might be something to add to this PR.
There was a problem hiding this comment.
@AshGDS have expanded the documentation in an extra commit, can you take a look?
andysellick
force-pushed
the
cw-add-rel
branch
from
6567c29 to
50dfd50
Compare
- explain difference between a set and add method for component options
andysellick
force-pushed
the
cw-add-rel
branch
from
50dfd50 to
0fd801e
Compare
What
Adds the rel attribute to the component wrapper helper.
Why
Needed for the migration of the button component onto the component wrapper.
Visual Changes
None.
Trello card: https://trello.com/c/hUDC8lzz/418-add-component-wrapper-helper-to-button-component