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

Add AppInstallPage #12122

Merged
merged 25 commits into from
Apr 5, 2024
Merged

Add AppInstallPage #12122

merged 25 commits into from
Apr 5, 2024

Conversation

danielfmiranda
Copy link
Collaborator

@danielfmiranda danielfmiranda commented Mar 28, 2024

Description

Link to sample test Youtube page: https://foundation-s-tiktok-pag-1mbxqc.mofostaging.net/en/youtuberegrets/
Link to sample test TikTok page: https://foundation-s-tiktok-pag-1mbxqc.mofostaging.net/en/tiktok/
Link to Figma File: Figma
Related PR: #12036

This PR introduces a new page model AppInstallPage which is planned to be used on the upcoming TikTok Reporter App page, and to also recreate the Youtube Regrets Reporter Extension page.

The ask for this page was to have the hero section of the regrets reporter page (with some minor design changes), and the "body/cta" section of a campaign page.

To achieve this I made the AppInstallPage inherit from our existing CampaignPage model, so they can share the same body/cta fields.

I also had app_install_page.html extend campaign_page.html, with some minor updates to accommodate requests from design.

Screenshots

Figma Mockup:

image

Review App:

Please note that:

  • I uploaded button icons that do not match the figma mock up, this is not an error.
  • The circular "share" button underneath the download buttons in the figma mockup is missing on the review app, this is going to be added at a later date.

Screenshot 2024-03-29 at 00-58-10 TikTok

┆Issue is synchronized with this Jira Story

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 28, 2024 21:34 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 28, 2024 22:03 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 28, 2024 22:11 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 28, 2024 22:27 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 02:01 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 02:13 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 02:25 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 02:26 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 07:44 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-qneczb March 29, 2024 07:53 Inactive
@danielfmiranda danielfmiranda changed the title Tiktok page Adding AppInstallPage for TikTok & YT Extension Pages Mar 29, 2024
@danielfmiranda danielfmiranda changed the title Adding AppInstallPage for TikTok & YT Extension Pages Add AppInstallPage Mar 29, 2024
@danielfmiranda danielfmiranda marked this pull request as ready for review March 29, 2024 08:17
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 3, 2024 19:53 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 3, 2024 19:54 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 3, 2024 20:01 Inactive
Copy link

@kristinashu kristinashu left a comment

Choose a reason for hiding this comment

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

So amazing! Thank you @danielfmiranda for getting all the little details of this right!

@kristinashu
Copy link

I'm sorry Daniel, feel free to do this as a follow up ticket. I just got the the final graph from the illustrator and it seems like the image is a little pixelated on the site. Is the image getting compressed by the cms? The original is 383kb which I feel is a reasonable size for a large hero banner. AA_FF_1730x567

Copy link
Collaborator

@robdivincenzo robdivincenzo left a comment

Choose a reason for hiding this comment

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

Excellent work @danielfmiranda ! 🔥

@simont-cr
Copy link
Collaborator

simont-cr commented Apr 4, 2024

@danielfmiranda While reviewing the app, I noticed two issues regarding buttons:

1. Mobile view buttons width
When viewing the app on mobile view, the buttons do not have a consistent width. I feel that it would be much better if they are a similar width, maybe matching the bigger button.

image

2. When using tab for navigation and onFocus, the text turns width on the buttons
Right now when using tab to navigate through the page, whenever the focus is any of the two buttons, the text turns into white but the button stays with the white background.

The onFocus styling should be similar as the onHover, which turns the button background color black.

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 4, 2024 22:06 Inactive
@danielfmiranda
Copy link
Collaborator Author

danielfmiranda commented Apr 4, 2024

Great catches @simont-cr!

My latest commit to this PR updates the width and focus state of the buttons 👍

Whenever you find a free moment, can you please take another look at the review apps and approve the PR if everything looks OK?

Thanks!

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 5, 2024 00:36 Inactive
@danielfmiranda
Copy link
Collaborator Author

Thanks for calling that out @kristinashu! I have created https://mozilla-hub.atlassian.net/browse/TP1-368 to investigate 👍

Copy link
Collaborator

@simont-cr simont-cr left a comment

Choose a reason for hiding this comment

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

Tabbing through the buttons no longer cause issues and mobile btns are now similar width as expected, looking good, thanks!

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-tiktok-pag-1mbxqc April 5, 2024 18:11 Inactive
Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

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

Thanks @danielfmiranda !

@danielfmiranda danielfmiranda merged commit f9c2297 into main Apr 5, 2024
6 checks passed
@data-sync-user
Copy link
Collaborator

➤ Simon Acosta Torres commented:

PR has already been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants