canonical / microstack.run

Code for the microstack.run website by Canonical.
https://microstack.run
5 stars 22 forks source link

[Sunbeam GA Launch] /index revamp #231

Closed mtruj013 closed 1 year ago

mtruj013 commented 1 year ago

Done

Issue / Card

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

webteam-app commented 1 year ago

Demo starting at https://microstack-run-231.demos.haus

codecov[bot] commented 1 year ago

Codecov Report

Merging #231 (7e09838) into main (7d5b65f) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 7e09838 differs from pull request most recent head bfd1039. Consider uploading reports for the commit bfd1039 to get more accurate results

@@           Coverage Diff           @@
##             main     #231   +/-   ##
=======================================
  Coverage   77.27%   77.27%           
=======================================
  Files           2        2           
  Lines          44       44           
=======================================
  Hits           34       34           
  Misses         10       10           
Flag Coverage Δ
python 77.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

juanruitina commented 1 year ago
mtruj013 commented 1 year ago

Thanks @juanruitina, I believe I've addressed all of your comments if you could take another look?

juanruitina commented 1 year ago

@tytus-kurek and I still need to agree on the label for the Discourse link on the nav, but other than that it looks good!

mtruj013 commented 1 year ago

@julpal7

juanruitina commented 1 year ago

Tytus and I have agreed on the navigation changes, they are in the copy doc. Can you please implement them?

julpal7 commented 1 year ago

The [ Download MicroStack Datasheet ] button is missing in the demo, but I see an issue with it has already been addressed in the copy doc, so I’m assuming it will get updated.

There is an unnecessary line under the header, it needs to be removed (it's more visible in the demo):

Screenshot 2023-05-31 at 17 10 05

There is an HR divider missing between the sections here:

Screenshot 2023-05-31 at 16 57 13

Between those sections as well:

Screenshot 2023-05-31 at 17 02 56

I put comments on the dividers in the copy doc. Apart from those minor issues, everything looks okay. @mtruj013 Thanks!

mtruj013 commented 1 year ago

Thanks @juanruitina and @julpal7 ! I've addressed your comments if you could both take another look? Julia I think the line under the suru is a vanilla bug specific to your browser, it isn't in the code and I can't see it locally or in the demo on either chrome or firefox

julpal7 commented 1 year ago

@mtruj013 Everything looks good now. The line is showing for me in Chrome, but as long as it's not in the code, it must be a bug on my side.

ClementChaumel commented 1 year ago

@julpal7 is your chrome zoomed in by any chance? I can see the line on both 110% and 90% zoom but not on 100%

julpal7 commented 1 year ago

@ClementChaumel you're a genius! I was on 90% for some reason, I reset the zoom and it's gone

juanruitina commented 1 year ago

LGTM!