canonical / ubuntu.com

The official website for the Ubuntu operating system
https://ubuntu.com
Other
201 stars 199 forks source link

Align macros usage with Vanilla standards #14166

Closed britneywwc closed 2 months ago

britneywwc commented 2 months ago

Done

To-do

QA

Issue / Card

Fixes WD-14119

Screenshots

[If relevant, please include a screenshot.]

Help

QA steps - Commit guidelines

webteam-app commented 2 months ago

Demo

Jenkins

demos.haus

britneywwc commented 2 months ago

Hey @jmuzina @pastelcyborg! I will be addressing comments of the macros usage in this PR. Is there a suggested way to import macros? The Vanilla build does not include macro files thus I'm not able to import them through npm, I'm currently just copy-pasting the macro codes directly.

pastelcyborg commented 2 months ago

Hey @jmuzina @pastelcyborg! I will be addressing comments of the macros usage in this PR. Is there a suggested way to import macros? The Vanilla build does not include macro files thus I'm not able to import them through npm, I'm currently just copy-pasting the macro codes directly.

Our intended approach at this point will just be copying the macros from the node_modules directory into the templates/macros directory of the project that's importing Vanilla, assuming said project is using Flask Base. For the sake of futureproofing, I'd just make sure your macros are in templates/macros. When we have more a more fleshed-out importing process, I'll drop you a line and let you know exactly what that looks like.

britneywwc commented 2 months ago

Thanks for all the suggestions! I have addressed all the comments in this PR and the feature branch, I've also consolidated them in the list of "To-do" under the PR description.

As for the quotes, I will wait for the Vanilla PR to be merged since the quotes that are being used in the case studies are the same.

jmuzina commented 2 months ago

Heads up @britneywwc : https://github.com/canonical/vanilla-framework/pull/5275 has macro markup that is ready for use, but there are a couple issues related to HR spacing that are blocking it - they will be solved by https://github.com/canonical/vanilla-framework/pull/5241.

jmuzina commented 2 months ago

Hi @britneywwc, if you have any thoughts about any of your experience using or creating macros as part of this work, it would be helpful to document them so we can work to improve them.

If that’s the case, could you record your thoughts in a Google Doc or something similar so we can address any issues? Thanks!!

britneywwc commented 2 months ago

Sure thing @jmuzina, I'll try to consolidate it in a google doc and send it over.

lyubomir-popov commented 2 months ago

image

image

image

Before:

image

After:

image

image

image

image

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.63%. Comparing base (5c64f62) to head (887b958). Report is 15 commits behind head on case-study-templates.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## case-study-templates #14166 +/- ## ===================================================== Coverage 69.63% 69.63% ===================================================== Files 109 109 Lines 3129 3129 Branches 1092 1092 ===================================================== Hits 2179 2179 Misses 925 925 Partials 25 25 ```
lyubomir-popov commented 2 months ago

Can we move these 3 cols ot the left, so the body content aligns with the body copy of the article underneath? image

also, are the h2s always going to be always this short?

pastelcyborg commented 2 months ago

@britneywwc Another thought here - if you want to be able to use the Hero macro without copy-and-pasting it into your project, you can make a fairly minor modification to the server config, as seen in this PR:

https://github.com/canonical/ubuntu.com/pull/14209

I'll leave it up to you if you'd like to - might save you a bit of effort/maintenance.

britneywwc commented 2 months ago

@lyubomir-popov For the highlights example, yes the h2 will only be "Highlights"

lyubomir-popov commented 2 months ago

image

jmuzina commented 2 months ago

Pushed a couple of quote adjustments from code & design review this morning with @pastelcyborg and @lyubomir-popov.