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-17353 Refresh home page #360

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

WD-17353 Refresh home page #360

wants to merge 6 commits into from

Conversation

minkyngkm
Copy link
Contributor

@minkyngkm minkyngkm commented Dec 17, 2024

Done

Refresh index.html

QA

Issue / Card

Fixes #https://warthogs.atlassian.net/browse/WD-17353

@webteam-app
Copy link

</tr>
<tr>
<th><strong>Android versions</strong></th>
<td data-heading="Appliance">10, 11, 12, 13</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be 12, 13 and 14 as covered in the copy doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated 👍

@minkyngkm minkyngkm force-pushed the WD-17353 branch 2 times, most recently from 4586c6b to 29fa765 Compare December 18, 2024 02:01
@minkyngkm
Copy link
Contributor Author

Hey @mattea-turic, the page is ready to review. Could I get a design review please? 🙏 A demo link is here: https://anbox-cloud-io-360.demos.haus/

@mattea-turic
Copy link

mattea-turic commented Dec 18, 2024

Thank you @minkyngkm ! Pretty much perfect:

For the testimonials section:

  • For Jacob Smith's role, could you add a br after "Packet", so that the entirety of the text in the parentheses occupies a new line

I'll add a +1 !

@minkyngkm
Copy link
Contributor Author

Thanks @mattea-turic, done! 👍

@Sophie-32
Copy link

Hi @minkyngkm, there are a few small items including:

  1. corrections on the copydoc for you.
  2. the three sub-headers should be h3
    Screen Shot 2024-12-18 at 3 16 22 PM
  3. make sure images have the alt=' ' so screen readers know they're decorative

@minkyngkm
Copy link
Contributor Author

Thanks @Sophie-32 for your review. I've applied your suggestions. Hope it looks better now!
image

Comment on lines +496 to +504
{{ image (
url="https://assets.ubuntu.com/v1/8b62755e-equinix-logo-vw.png",
alt="Equinix logo",
width="80",
height="100",
hi_def=True,
loading="lazy"
) | safe
}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattea-turic I couldn't find this image in the Figma file from the asset server. I used the same logo from the logo section above but resized it to make it smaller. Would this be okay?
image

@Sophie-32
Copy link

Sophie-32 commented Dec 19, 2024

@minkyngkm I apologize I think I was unclear:

  • I marked the heading tags for the 'Why Anbox cloud?' in the copydoc using suggested mode
    Screen Shot 2024-12-19 at 5 22 20 PM

  • These images still don't have the alt text for decorative images
    Screen Shot 2024-12-19 at 5 23 17 PM

Let me know if either of those is unclear

@minkyngkm
Copy link
Contributor Author

Thank you @Sophie-32 🙏

Updated the headings
image

All images showed alt to me so I didn't make any changes to the alts
image

@eliman11
Copy link

eliman11 commented Jan 7, 2025

Thanks @minkyngkm - commenting in place of Sophie while she's away. I can't seem to view the demo now but I think Sophie was suggesting to remove the alt text and replace it with alt=' '. Since the images are decorative, alt text is not necessary for screen readers. Thank you for your help on this!

@minkyngkm
Copy link
Contributor Author

@eliman11 Thanks, I've removed the alt texts for the decorative images. Demo should be working at https://anbox-cloud-io-360.demos.haus/ 🙏

@eliman11
Copy link

eliman11 commented Jan 8, 2025

Looks great, thank you for the revisions! Adding UX+1

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.

7 participants