Update Strapi page title with background colour and I belong flag #2332
Conversation
msquance-stem
left a comment
msquance-stem
left a comment
There was a problem hiding this comment.
Looks good, but I think we should bring the styles controlling the flag into the component, as it will be easier to find in the future, and then you don't have to worry about something external to the component changing it
| <%= render Cms::ImageComponent.new(@title_image, show_caption: false) %> | ||
| </div> | ||
| <% elsif @i_belong_flag %> | ||
| <div class="page-title__media i-belong-hero__area--logo"> |
There was a problem hiding this comment.
Should we change these classes to reflect the component, and then replicate the scss code into here, rather than relying on the global styles, as I think we should move away from them.
| @i_belong_flag = i_belong_flag | ||
| end | ||
|
|
||
| def background_color_class |
There was a problem hiding this comment.
I realised we weren't setting a default background colour, existing pages using the component wouldn't have a background colour class. This method sets it to the default lime green.
There was a problem hiding this comment.
Could you just change the component definition to have "lime-green" as the default for the background color param? Saves you needing the method
msquance-stem
left a comment
msquance-stem
left a comment
There was a problem hiding this comment.
LGTM, one suggestion that would simplify your default colour
| @i_belong_flag = i_belong_flag | ||
| end | ||
|
|
||
| def background_color_class |
There was a problem hiding this comment.
Could you just change the component definition to have "lime-green" as the default for the background color param? Saves you needing the method
Updated mocks Added graphql query test
Moved I belong flag css into component Updated test to cover new method
msquance-stem
force-pushed
the
2995-strapi-component-update---page-title
branch
from
8a40b4d to
106376f
Compare
|
Status
Review progress:
What's changed?
Steps to perform after deploying to production
If the production environment requires any extra work after this PR has been deployed detail it here. This could be running a Rake task, migrating a DB table, or upgrading a Gem. That kind of thing.