Skip to content
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

feat: Build /20-04 page #14559

Merged
merged 7 commits into from
Dec 20, 2024
Merged

feat: Build /20-04 page #14559

merged 7 commits into from
Dec 20, 2024

Conversation

petesfrench
Copy link
Contributor

@petesfrench petesfrench commented Dec 11, 2024

Done

  • Build /20-04 page based off of copydoc and figma design
  • Added some features to _latest_news_strip.html
  • Bumped vanilla to 4.18.4

QA

  • Check out demo link
  • Check the design matches the figma design
  • Check the content matches the copydoc
  • Check there is correct application of vanilla, specifically focusing on macro usage
  • Check on different screen sizes
  • Check the pages where _latest_news_strip.html is implemented and ensure there is no difference

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-17287

@webteam-app
Copy link

@mattea-turic
Copy link
Collaborator

mattea-turic commented Dec 11, 2024

Thanks @petesfrench ! :) A few comments from me:

For the hero section section:

  • Could you add a <br> after "support" pls?

For the "Expanded Security Maintenance (ESM) for 20.04 LTS" section:

  • Could you replace the current full-width image under ESM with this one? PM got back to me and is happy with the proposed change.

For the "What do you get with Ubuntu Pro for 20.04 LTS?" section:

  • The CTA to the blog post can be removed (spoke to Sophie about this and it was a mistake in the file)
  • Could you also add a HR above the first list item in the "Application packages" subsection?

For the "What customers say about ESM" section:

  • There looks to be excess padding between the two testimonials – could you wrap the first testimonial in just a shallow pls?
  • And for smaller screens, could you remove the shallow as I think this might help a little with hierarchy.
  • I'd also say (again, for smaller screens) remove the hr above the person's name & role, though iirc Madhur mentioned that this is something that isn't easily fixed as it's embedded within the comp

For the blog section:

  • Could you apply deep padding there, since it's the last section of the page?
  • For tablets, can the items span full width as they're right-aligned atm with empty columns

And a quick question about tables on smaller screens:
Screenshot 2024-12-11 at 17 31 33
Is there something we can do in place of truncation? No worries if not, just wanted to check with you

@mattea-turic
Copy link
Collaborator

Looks great, ty @petesfrench ! I'll +1, but just wanted to ask if you could add the line break after "support" in the h2 of the hero?

templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved

{%- if slot == 'heading_link' -%}
<div class="p-section--shallow">
<a href="https://ubuntu.com/engage">All the success stories&nbsp;&rsaquo;</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find this link on the copy doc, is it missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattea-turic Where is this supposed to link to? I just guessed with this link

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petesfrench Took from UX wireframe, I see now it wasn't in the copydoc, so I think you're good to just delete it. I'll just double-check with Sophie

templates/20-04/index.html Outdated Show resolved Hide resolved
britneywwc
britneywwc previously approved these changes Dec 17, 2024
Copy link
Contributor

@britneywwc britneywwc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great thanks, +1 with some minor suggestions

templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
templates/20-04/index.html Outdated Show resolved Hide resolved
@Sophie-32
Copy link

Screen Shot 2024-12-18 at 2 30 22 PM

Will the capital H2 read the same as lowercase h2? Asking because this is the only tag thats capitalized.

@Sophie-32
Copy link

missing alt= for the images

@Sophie-32
Copy link

I pointed it out to PM, but they have two copydocs and right now the one linked here is not the correct one. This is the correct one and is the one linked in the JIra issue - https://docs.google.com/document/d/1lzxAL9Wt24m5-p5qG9y190rOg3FY5zVdKLYLlBoBcLU/edit?tab=t.0

@petesfrench
Copy link
Contributor Author

Screen Shot 2024-12-18 at 2 30 22 PM

Will the capital H2 read the same as lowercase h2? Asking because this is the only tag thats capitalized.

This is not capitalized for me in code or in the demo. I think it might be your browser

@petesfrench
Copy link
Contributor Author

missing alt= for the images

All the images have alt="", is this not the correct practice to follow?

@petesfrench petesfrench merged commit 901b349 into main Dec 20, 2024
29 checks passed
@petesfrench petesfrench deleted the wd-17287 branch December 20, 2024 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants