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

Intel NUC desktop rebrand #13570

Conversation

carkod
Copy link
Contributor

@carkod carkod commented Feb 12, 2024

QA

Issue / Card

Fixes https://ubuntu-com-13570.demos.haus/

Screenshots

managed

Help

QA steps - Commit guidelines

@webteam-app
Copy link

webteam-app commented Feb 12, 2024

@carkod carkod marked this pull request as draft February 12, 2024 18:22
@carkod carkod changed the title Intel nuc desktop rebrand wd 8799 WIP Intel nuc desktop rebrand wd 8799 Feb 12, 2024
@carkod carkod force-pushed the intel-nuc-desktop-rebrand-wd-8799 branch from d116d99 to 62d5ade Compare February 12, 2024 18:27
@carkod carkod changed the title WIP Intel nuc desktop rebrand wd 8799 WIP Intel NUC desktop rebrand Feb 12, 2024
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8bb01a1) 74.41% compared to head (4ce4189) 74.41%.

Additional details and impacted files
@@                   Coverage Diff                    @@
##           download-bubble-wd-8482   #13570   +/-   ##
========================================================
  Coverage                    74.41%   74.41%           
========================================================
  Files                          107      107           
  Lines                         2838     2838           
  Branches                       946      946           
========================================================
  Hits                          2112     2112           
  Misses                         702      702           
  Partials                        24       24           

@carkod carkod force-pushed the intel-nuc-desktop-rebrand-wd-8799 branch 2 times, most recently from 94f00ec to 8902c8e Compare February 13, 2024 10:34
@carkod carkod changed the title WIP Intel NUC desktop rebrand Intel NUC desktop rebrand Feb 13, 2024
@juanruitina
Copy link
Contributor

juanruitina commented Feb 13, 2024

The sections under "Next steps" don't exist on the live version, so please remove. I've adjusted the copy doc accordingly, please flag this up when it happens.

The form and the footer are missing, can you please incorporate them before I do a full review? Thank you.

@lyubomir-popov
Copy link
Contributor

  • wrong style here - please use an hr.p-rule above this, and make it an .p-section>h2 followed by the image

image

  • pls strip the background from this, see the design:
    image

(review not finished, will continue tomorrow)

@carkod carkod force-pushed the intel-nuc-desktop-rebrand-wd-8799 branch 4 times, most recently from d47af01 to 0dc4412 Compare February 13, 2024 22:16
@carkod
Copy link
Contributor Author

carkod commented Feb 13, 2024

  • wrong style here - please use an hr.p-rule above this, and make it an .p-section>h2 followed by the image

image

  • pls strip the background from this, see the design:
    image

(review not finished, will continue tomorrow)

Addressed this comment and the ones in MM

@carkod carkod marked this pull request as ready for review February 14, 2024 09:50
@lyubomir-popov
Copy link
Contributor

Hi,

  • as I mentioned on mattermost, the lines missing above list items on smaller breakpoints:
    image

  • stepped lists have 0 in the counter - no need for that:
    image

  • pls take this from MariaPaula's pr - this doesn't look like in the design:

image

and finally, might be worth using the same components - row--50-50 for example for the first section.

I'm pushing a fix - the first section should have .p-strip is-shallow.u-no-padding--bottom>.p-section; next release of vanilla will have p-section--hero that does all that with one class

@carkod carkod force-pushed the intel-nuc-desktop-rebrand-wd-8799 branch 2 times, most recently from 18d9b3e to 27bbc38 Compare February 14, 2024 19:35
@juanruitina
Copy link
Contributor

UX+1

Copy link
Contributor

@mtruj013 mtruj013 left a comment

Choose a reason for hiding this comment

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

LGTM, with a couple of minor changes

@carkod carkod force-pushed the intel-nuc-desktop-rebrand-wd-8799 branch from d51b66b to 4ce4189 Compare February 21, 2024 18:53
@carkod
Copy link
Contributor Author

carkod commented Feb 21, 2024

Checked, this isn't changing templates/download/intel-iei-tank-870.html, so merging.

@carkod carkod merged commit daf79af into canonical:download-bubble-wd-8482 Feb 21, 2024
14 of 15 checks passed
@carkod carkod deleted the intel-nuc-desktop-rebrand-wd-8799 branch February 21, 2024 18:58
akbarkz pushed a commit that referenced this pull request Feb 23, 2024
akbarkz pushed a commit that referenced this pull request Feb 27, 2024
akbarkz pushed a commit that referenced this pull request Mar 4, 2024
carkod added a commit to carkod/ubuntu.com that referenced this pull request Mar 11, 2024
akbarkz pushed a commit that referenced this pull request Mar 12, 2024
akbarkz pushed a commit that referenced this pull request Mar 20, 2024
akbarkz pushed a commit that referenced this pull request Apr 1, 2024
akbarkz pushed a commit that referenced this pull request Apr 8, 2024
mtruj013 pushed a commit to mtruj013/ubuntu.com that referenced this pull request Apr 8, 2024
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