canonical / juju.is

The website for juju.is
GNU General Public License v3.0
7 stars 26 forks source link

Wd 9788 juju feature pages #525

Closed abbiesims closed 6 months ago

abbiesims commented 6 months ago

Continues on from https://github.com/canonical/juju.is/pull/524

Done

Adds content for Juju middle pages and includes @chillkang 's existing changes.

QA

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-9773 Fixes https://warthogs.atlassian.net/browse/WD-9788 Fixes https://warthogs.atlassian.net/browse/WD-9778 Fixes https://warthogs.atlassian.net/browse/WD-9783 Fixes https://warthogs.atlassian.net/browse/WD-9768.

webteam-app commented 6 months ago

Demo

Jenkins

demos.haus

bartaz commented 6 months ago

A bit unrelated to this PR, on smaller screen sizes navigation items start wrapping:

image

It seems you need to adjust the $breakpoint-navigation-threshold value for it not to happen (nav should switch to mobile view before items wrap).

Related to this PR you can notice that on smaller screen sizes tabs are no longer aligned with top nav:

image

It's because they don't use grid, but hardcoded padding value. This may be ok as MVP, but for it to fully work across screen sizes there would need to be some bigger changes to the tabs (to respect the grid layout).

lukasSerelis commented 6 months ago

Landing page section still missing button to get to the middle pages (outside scope?) DEMO image Figma image

Image on juju architecture stretched DEMO image Figma image

2nd image on juju architecture is too bright (link to darker version https://drive.google.com/file/d/1NlMx3z2c9YVWLz1fsH7p52FQvAT_pSvR/view?usp=drive_link) DEMO image Figma image

Text on charm architecture takes too many columns DEMO image Figma image

chillkang commented 6 months ago

A bit unrelated to this PR, on smaller screen sizes navigation items start wrapping:

image

It seems you need to adjust the $breakpoint-navigation-threshold value for it not to happen (nav should switch to mobile view before items wrap).

Related to this PR you can notice that on smaller screen sizes tabs are no longer aligned with top nav:

image

It's because they don't use grid, but hardcoded padding value. This may be ok as MVP, but for it to fully work across screen sizes there would need to be some bigger changes to the tabs (to respect the grid layout).

@bartaz I'll try to address the $breakpoint-navigation-threshold value issue in another PR! For the tabs, this is a temporary workaround and I was thinking opening a new PR to make a new variant, e.g., p-tabs--25-75, that you talked about the other day.

lukasSerelis commented 6 months ago

Updated this image to scale keep better quality and have a consistent background https://drive.google.com/file/d/1PuLtgqRGrOjpVUPkYjW1q4jhAZ3ySDJ0/view?usp=drive_link image

jnsgruk commented 6 months ago

Big improvement overall, I spotted some weird padding on the charms architecture page in one of the tables:

image

chillkang commented 6 months ago

@bartaz @jnsgruk @lukasSerelis Could you have a look at the changes? Thanks in advance!

lukasSerelis commented 6 months ago

LGTM :+1:

bartaz commented 6 months ago

Can't find it in code (maybe not affected by this PR?) but this section at the bottom seems like it could use p-section--deep for more spacing.

image
codecov[bot] commented 6 months ago

Codecov Report

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

Project coverage is 58.51%. Comparing base (44a69e0) to head (043a8fb). Report is 12 commits behind head on main.

:exclamation: Current head 043a8fb differs from pull request most recent head 5ab2c68. Consider uploading reports for the commit 5ab2c68 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #525 +/- ## ======================================= Coverage 58.51% 58.51% ======================================= Files 6 6 Lines 188 188 ======================================= Hits 110 110 Misses 78 78 ``` | [Flag](https://app.codecov.io/gh/canonical/juju.is/pull/525/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | Coverage Δ | | |---|---|---| | [python](https://app.codecov.io/gh/canonical/juju.is/pull/525/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `58.51% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lukasSerelis commented 6 months ago

@massigori changed why juju image, waiting for :+1: so @chillkang could change it image https://drive.google.com/file/d/1gKIsJ1RXK2d283FEtwWXBpI2jRlBDliY/view?usp=drive_link