-
Notifications
You must be signed in to change notification settings - Fork 5
(#5025) Blog Post Mock HTML #2091
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
base: develop
Are you sure you want to change the base?
Conversation
Viewing Information |
c47ef16 to
83bbb05
Compare
c61f767 to
2829efc
Compare
bryanpizzillo
left a comment
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.
Ok, a few things to get you started.
On over all note - the whole blog post is not a BEM element. So all the sub components (Categories, archives, etc) should be their own BEM elements. I noted about the blog post items below, but remember Categories and archives ARE ALSO FOR SERIES. So they should be something like cgdp-blog-archive and cgdp-blog-categories and definitely NOT cgdp-blog-post__categories.
This is good example for @andyvanavery31 why I had said we should mock both the blog post and series. So you should really add series to this as the templates are shared.
| <main id="main-content" class="contentzone"> | ||
| <div> | ||
| <article> | ||
| <div class="resize-content"> |
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.
Instead of going through the HTML with what to keep and what to kill here is what the content should be like... I followed the work that was done for all the other inner pages.
<article>
<h1 class="nci-heading-h1">{title}</h1>
{{ NEW - Subscribe and Byline/date }}
<div class="cgdp-inner-content-area">
{{ field_image_article }}
{{ field_intro_text }}
<div class="cgdpl>
<div class="contentzone">
<div id="cgvBody">
{{body}}
</div>
</div>
</div>
</div>
<footer class="cgdp-article-footer">
{{ field_citation }}
{{ field_related_resources }}
{{ NEW - PAGER HERE }}
{{ public_use }}
</footer>
</article>
Also, nothing should be cgdp-blog-post. We are not going that crazy... The 2 styles we follow for custom bits are:
- Fields can be
cgdp-field-<field_name>e.g.,cgdp-field-intro-text. This is used when a field is on its own. - Make a new BEM block like the Press Release Title Block. In this case the Page Title had a subtitle, and the dates appeared at the top, and the there is a weird contact us box.
- You probably should take this approach for the subscribe/byline/date. The title should be able to stay in the template as I have it above -- PR title got junk above it, thus could not hang out on its own. So maybe, IDK,
cgdp-blog-post-infoas you BEM element to hold the subscribe link and byline/date.
- You probably should take this approach for the subscribe/byline/date. The title should be able to stay in the template as I have it above -- PR title got junk above it, thus could not hang out on its own. So maybe, IDK,
Templates -
| import footer from './blog-footer'; | ||
|
|
||
| const html = ` | ||
| <main id="main-content" class="contentzone"> |
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 am iffy on <main> being here or in the main template. It should probably go into the thing you are using for the 2 columns, wrapping the left column where this is placed.
| @@ -0,0 +1,29 @@ | |||
| const footer = ` | |||
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.
See my notes in blog-content that apply to the footer area. But:
- The footer class is
cgdp-article-footer - public use already exists and you should follow its classes
- I also noted on the ticket that the comp is missing citations and related resources.
- The pager should be something like
cgdp-blog-post-pager(it is its own BEM element) (Also do you need the clearfix as this will be outside of.cgdpl??)
| import imageLead from './img/hug.jpg'; | ||
|
|
||
| export const leadImage = ` | ||
| <div data-entity-embed-display="view_mode:media.image_display_article_medium" class="embedded-entity align-right"> |
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.
So this is the old stuff. It needs to be the new image. I am going to assume the cgdp-image HTML you have below is correct, but this needs to be wrapped with the HTML in the field_image_article template I linked above. (TL;DR; <div class="cgdp-field-image-article"> should replace the <div data-entity-embed... you have above.
5cd8449 to
ababc45
Compare
8944af9 to
fb24d45
Compare
ee9e1f1 to
9afaa9b
Compare
Closes #5025 Closes #5070
9afaa9b to
217dbab
Compare
Closes 5070
Closes 5025
Pull Request Details
Add description
Closes #
Author PR Checklist
Items that the author of the PR is responsible for checking before submitted the PR.
General:
Accessibility:
Development:
Product Reviewer PR Checklist
Items the product team is responsible for reviewing.
General:
Accessibility:
Design Reviewer PR Checklist
Items the design team is responsible for reviewing.
General:
Developer Reviewer PR Checklist
Items the development team is responsible for reviewing.
General:
Accessibility: