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

C3-713: Redesign public representation for certified machines #14135

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

andrejvelichkovski
Copy link
Contributor

@andrejvelichkovski andrejvelichkovski commented Aug 1, 2024

Done

  • Updated the design for each certified machine on the public website:
    • Motivation: the older design was outdated and not very representative for our work
  • Implemented new view for presenting platforms. Platform is a collection of multiple machines, sharing the base components. Each platform might have more than one machine belonging to it (see example screenshots below).

The implementation is based on this design:

Screenshot from 2024-08-20 16-57-03

Demo examples

QA

  • Check out this feature branch
  • Run the site using the command ./run serve or dotrun
  • View the site locally in your web browser at: http://0.0.0.0:8001/
    • Be sure to test on mobile, tablet and desktop screen sizes
  • [List additional steps to QA the new features or prove the bug has been resolved]

Issue / Card

Fixes C3-713

Screenshots

Machine page

Screenshot from 2024-08-20 14-46-06

Platform page

Screenshot from 2024-08-20 14-46-20

Help

QA steps - Commit guidelines

@webteam-app
Copy link

andrejvelichkovski is not a collaborator of the repo

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.63%. Comparing base (dcc3d37) to head (8f30cc4).
Report is 29 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14135   +/-   ##
=======================================
  Coverage   69.63%   69.63%           
=======================================
  Files         120      120           
  Lines        3418     3418           
  Branches     1174     1174           
=======================================
  Hits         2380     2380           
  Misses       1013     1013           
  Partials       25       25           

see 1 file with indirect coverage changes

@nadzyah
Copy link

nadzyah commented Aug 14, 2024

The test_model_details test fails because we're changing the API as well since we need more information from the server to implement the design.

@akbarkz
Copy link
Contributor

akbarkz commented Aug 20, 2024

The demo instance link: https://ubuntu-com-14135.demos.haus/

@andrejvelichkovski andrejvelichkovski changed the title C3-713: Implement Platform Page C3-713: Redesign public representation for certified machines Aug 20, 2024
@andrejvelichkovski andrejvelichkovski marked this pull request as ready for review August 21, 2024 07:19
@andrejvelichkovski
Copy link
Contributor Author

Converting this PR to draft as we are still awaiting on feedback from different partners.

@andrejvelichkovski andrejvelichkovski marked this pull request as draft August 22, 2024 11:27
@andrejvelichkovski andrejvelichkovski marked this pull request as ready for review September 3, 2024 15:46
@juanruitina
Copy link
Contributor

Glad to see it in such good shape. Just very minor comments, all for the machine pages:

  •  Change the button text for the vendor link to "Visit the vendor website" to make it clear what we are doing here
  • In here, can you correctly capitalise "Notebook" in the hero H2?
  • Please only show the tabs if there's more than one release. (It's OK to still show them in the platform page, it's informative there; here it's redundant with the H2)
  • Change the second column in the two spec tables from colspan="7" to colspan="6", that way they will align nicely with the grid and the "Issues? Let us know" section.
  • To make the page lighter, I think we can do without the "Notes" heading, see below. In the end, the paragraph above is also a note. (Please also remove u-no-margin--bottom from the paragraph above)

Screenshot 2024-09-04 at 13 43 23

+1ing this already as to not block.

@andrejvelichkovski
Copy link
Contributor Author

andrejvelichkovski commented Sep 4, 2024

Thanks for the review @juanruitina

  • Change the button text for the vendor link to "Visit the vendor website" to make it clear what we are doing here

Sorry, I missed this, changed it now

  • In here, can you correctly capitalise "Notebook" in the hero H2?

This is a bit more complicated issue. To somewhat resolve it, I added a capitalize filter that should do this. However, the root cause is that a wrong data gets exposed from our APIs. We have an open ticket to fix this, so hopefully when we get that done the data itself will be in much better shape.

  • Please only show the tabs if there's more than one release. (It's OK to still show them in the platform page, it's informative there; here it's redundant with the H2)

Great suggestion, I now added an if clause to check this

  • Change the second column in the two spec tables from colspan="7" to colspan="6", that way they will align nicely with the grid and the "Issues? Let us know" section.

Done.

  • To make the page lighter, I think we can do without the "Notes" heading, see below. In the end, the paragraph above is also a note. (Please also remove u-no-margin--bottom from the paragraph above)

Done.

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