Strapi component - Horizontal card with asides #2338
Conversation
msquance-stem
left a comment
msquance-stem
left a comment
There was a problem hiding this comment.
Looks good, just one point that could be potentially refactored
| classes = ["horizontal-card-with-asides-component__wrapper white-bg"] | ||
| if @color_theme | ||
| classes << cms_color_theme_class(@color_theme, "left") | ||
| classes << "#{@color_theme}-theme" |
There was a problem hiding this comment.
I wonder if we should move this into the return of cms_color_theme_class method as I think we always need the two together?
I think we have this in a few places.
There was a problem hiding this comment.
Nice idea, I'll take a look at adding this in.
There was a problem hiding this comment.
Taking a look at this in more detail, I don't actually need the classes << "#{@color_theme}-theme" line.
I lifted this from the existing horizontal banner component, the class is only needed on that component as it applies the i belong flag when it has the i-belong-theme class applied.
New graphql query for the component Added testing Updated Nccebutton to have spacing above
Adding preview
…e in another branch
msquance-stem
force-pushed
the
2990-strapi-component---horizontal-card-with-aside
branch
from
7533171 to
430c33e
Compare
|
msquance-stem
deleted the
2990-strapi-component---horizontal-card-with-aside
branch
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.