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

WD-14930 Dev create /partner/intel #14568

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

immortalcodes
Copy link
Member

@immortalcodes immortalcodes commented Dec 13, 2024

Done

  • Dev created /partner/intel
  • Removed redirect

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
  • Demo
  • Copydoc
  • Design

Issue / Card

Fixes # WD-14930

Screenshots

[If relevant, please include a screenshot.]

Help

QA steps - Commit guidelines

@webteam-app
Copy link

@mattea-turic
Copy link
Collaborator

mattea-turic commented Dec 13, 2024

Thanks @immortalcodes ! :) A few comments:

For the Hero section:

  • I think I embedded the 3% BG with the image there, which is making it darker than it should be. I've uploaded a new one here

For the "Explore how to optimize performance for Intel platforms in our blog" section:

  • Could you add a full-width hr above pls?
  • Also, for smaller screens, the sizing looks a little off (620px - 1035px)
Screenshot 2024-12-13 at 11 22 32
  • Have got this problem for the "Explore Ubuntu-certified..." section too

For the "Explore Ubuntu-certified Intel Hardware backed by Ubuntu Pro" section:

  • The wrong images are being used here, so pls replace with the right ones: laptops, desktops, servers, and IoT devices
  • Layout problem on smaller screens, as mentioned earlier

For the "Want to optimize your silicon for Ubuntu?" section:

  • Could you add a hr above the first list item pls? This was my bad, as I hadn't included it in the figma file

@immortalcodes
Copy link
Member Author

Thanks @immortalcodes ! :) A few comments:

For the Hero section:

  • I think I embedded the 3% BG with the image there, which is making it darker than it should be. I've uploaded a new one here

For the "Explore how to optimize performance for Intel platforms in our blog" section:

  • Could you add a full-width hr above pls?
  • Also, for smaller screens, the sizing looks a little off (620px - 1035px)
Screenshot 2024-12-13 at 11 22 32 * [ ] Have got this problem for the "Explore Ubuntu-certified..." section too

For the "Explore Ubuntu-certified Intel Hardware backed by Ubuntu Pro" section:

  • The wrong images are being used here, so pls replace with the right ones: laptops, desktops, servers, and IoT devices
  • Layout problem on smaller screens, as mentioned earlier

For the "Want to optimize your silicon for Ubuntu?" section:

  • Could you add a hr above the first list item pls? This was my bad, as I hadn't included it in the figma file

Hey, regarding the sizing on smaller screens, is it related to alignment being a bit off? This is the default property of Vanilla and there is less that could be done for this, we could remove the vanilla class but that will make images bigger like in ss below or we can decide not to have images at all.
This was not the case with previous sections because they don't have a background and they mix with the entire container.
image

@mattea-turic
Copy link
Collaborator

mattea-turic commented Dec 16, 2024

we can decide not to have images at all

I think this might be the best option; if they're too big they'll distract from the text and force more scrolling

I'll +1! (once new demo is up)

@eliman11
Copy link

eliman11 commented Dec 16, 2024

Thanks @immortalcodes! A side note on the navigation for this page - I'll put in a separate request to edit the silicon partners page to to include links to relevant webpages and to add a menu for 'Partners' in the footer.

Comments:

  • Got confirmation re forms - Please link the 2 'Contact us' CTAs to a modal for marketo ID: 4205, the same form as on this page.
  • The 'Learn more' CTA under 'Want to optimize your silicon for Ubuntu?' is returning a 404 for me, could you please link it to https://canonical.com/partners/silicon?
  • Could you add h3 tags to the headings under the images for these two sections?
Screenshot 2024-12-16 at 10 37 26 Screenshot 2024-12-16 at 10 38 13

@eliman11
Copy link

eliman11 commented Dec 16, 2024

Thanks for your patience on this page @immortalcodes! I just checked with Gloria - the URL for this page should be canonical.com/partners/silicon/intel instead of ubuntu.com/partner/intel suggested on the copydoc. Would it be possible to make this change?

The stakeholder has also requested that we redirect ubuntu.com/download/intel and ubuntu.com/intel to this new page. Thanks!

Copy link
Contributor

@britneywwc britneywwc left a comment

Choose a reason for hiding this comment

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

I see that the stakeholders changed the page location from u.com to c.com, if that's the case then this page shouldn't live on u.com at all but only add redirects. Can we move this PR over to c.com?

@@ -674,7 +674,6 @@ packages/kde351.*/?: "https://releases.ubuntu.com/"
partners/contact-us/?: "https://partner-portal.canonical.com/"
Copy link
Contributor

@britneywwc britneywwc Dec 18, 2024

Choose a reason for hiding this comment

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

We should add redirects from /download/intel and /intel to the new page

templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Show resolved Hide resolved
templates/partner/intel.html Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
templates/partner/intel.html Outdated Show resolved Hide resolved
britneywwc
britneywwc previously approved these changes Dec 19, 2024
Copy link
Contributor

@britneywwc britneywwc left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes, just one more thing. I'll leave a +1 with changes

</section>

<section class="p-section">
<div class="row--50-50">
Copy link
Contributor

Choose a reason for hiding this comment

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

This row should be wrapped in a shallow section

@Sophie-32
Copy link

  • Add alt='' for all images since they are decorative
  • Please adjust one button label (see suggested text in the copydoc)

You can UX+1 with those changes.

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.

6 participants