-
Notifications
You must be signed in to change notification settings - Fork 45
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
DS-599: Optimized Campaign Landing Page #2310
Conversation
… icon to the Icon Element
…gnsystem/bolt into feature/DS-599-campaign-landing
…anding page templates, added a JS snippet to close the model on desktop
…ed the OCLP Video page with a Preview button
@mikemai2awesome I will like some feedback on the |
TODO: We need to solve the "Theme 4" and "Theme 6" image placements. It seems that these one are not rendering correctly on the template due to their image orientation (these are vertical while the remaining four are horizontal). We might need to re-export these assets to change their orientation. |
… the desktop form
@colbytcook Thank you for jumping on this. I'll review what you did. I don't think we need to change any assets, they are positioned with specific css in specific layouts. |
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.
Waiting for #2313 (review) to get the Video Thumbnail updates.
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.
I pulled in the Video Thumbnail updates. This is all set.
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.
This looks good guys! Tested new templates and Bolt Layout (no regressions found).
I added the Button and Link Text icon spacing changes to the Release Notes. Important to flag this type of change for QA.
@@ -8,7 +8,7 @@ | |||
{% set item_attributes = create_attribute(attributes|default({})) %} | |||
|
|||
<bolt-layout-item | |||
{{ source_order ? 'source-order="' ~ source_order ~ '"' : '' }} |
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.
Does this fix a bug? It looks like the name didn't match the schema before. If it fixes a Layout bug, let's note it in the Release Notes.
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.
Seems like it would fix a bug... we're using the <bolt-layout>
custom element in the layout authoring work, because twig wasn't working (I don't know the details). Maybe this was the problem, or part of it?
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.
Yes this was a fix to a bug in the ordering, there was a discrepancy between the schema and the twig (exactly what you said Dan).
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.
Broadly speaking, the code looks good. We plan to review these with stakeholders, right?
Jira
https://pegadigitalit.atlassian.net/browse/DS-599
Summary
Created the demo pages for the Optimized Campaign Landing Page
Details
How to test
Release notes
Visual Changes
Button Element
Text Link Element