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.

Sign up for GitHub

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

Jump to bottom

Navattic page blocks. #355

Merged
merged 8 commits into from
Nov 17, 2023
Merged

Navattic page blocks. #355

merged 8 commits into from
Nov 17, 2023

Conversation

nimithshetty17
Copy link

@nimithshetty17 nimithshetty17 commented Nov 8, 2023 •
edited
Loading

Issue

Fixes #MERATIVE-871

Description

This is a request made to improve/optimize the user experience of the Navattic demo
The demo page template currently lives within AEM, however, it needs to be moved into Franklin as part of the phase 3 migration effort.

Test URLs

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 8, 2023 •
edited
Loading

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 8, 2023

Page Scores Audits Google
/drafts/nimith/navattic-page Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 8, 2023

Page Scores Audits Google
/drafts/nimith/navattic-page Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 8, 2023

Page Scores Audits Google
/drafts/nimith/navattic-page PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@keith-kaplan
Copy link
Collaborator

@nimithshetty17 let's make the footer for the demos a Fragment since that shouldn't be changed by an author.

sachinmesh
sachinmesh suggested changes Nov 9, 2023
View reviewed changes
Copy link

@sachinmesh sachinmesh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  1. Horizontal product logo text is missing.
image
  1. The divider line color should be Neutral 140 i.e. 5C5A57
image
  1. Hamburger menu is missing on mobile.
image
  1. Footer font style on mobile should be 14px light
image

@nimithshetty17 @keith-kaplan @anabarcelona

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 14, 2023

Page Scores Audits Google
/drafts/nimith/navattic-page PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 14, 2023

Page Scores Audits Google
/drafts/nimith/navattic-page PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

nimithshetty17 commented Nov 14, 2023 •
edited
Loading

@keith-kaplan have created and added two fragments for the footer from the approach Charity had suggested and they can be viewed here:- https://navattic-page-blocks--merative2--nimithshetty17.hlx.page/drafts/nimith/navattic-updated.
Let me know if this is the expected approach.

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 14, 2023

Page Scores Audits Google
/drafts/nimith/navattic-updated PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

@sachinmesh the below issues are addressed:-

  1. Horizontal product logo text is missing. - Fixed
  2. The divider line color should be Neutral 140 i.e. 5C5A57 - Fixed
  3. Hamburger menu is missing on mobile. - Fixed
  4. Footer font style on mobile should be 14px light - Fixed

Please have a look and let me know if there are more.

@keith-kaplan
Copy link
Collaborator

@nimithshetty17 I created another demo for Micromedex and just saw that the CTA styling is broken when only using on button. Can we make sure the formatting is still correct even if one CTA is present?

https://navattic-page-blocks--merative2--nimithshetty17.hlx.page/clinical-decision-support/demo

@nimithshetty17
Copy link
Author

@nimithshetty17 I created another demo for Micromedex and just saw that the CTA styling is broken when only using on button. Can we make sure the formatting is still correct even if one CTA is present?

https://navattic-page-blocks--merative2--nimithshetty17.hlx.page/clinical-decision-support/demo

@keith-kaplan for solution it has to be authored by adding a field called CTA and the buttons below. Please check https://navattic-page-blocks--merative2--nimithshetty17.hlx.page/clinical-decision-support/demo.

@sachinmesh
Copy link

  1. On mobile and tablet, product logo should be 36px height.
image
  1. There should not be any vertical separator line after the logo.

@nimithshetty17 @keith-kaplan @anabarcelona

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 15, 2023

Page Scores Audits Google
/drafts/nimith/navattic-updated PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

  1. On mobile and tablet, product logo should be 36px height. - fixed
  2. There should not be any vertical separator line after the logo. - fixed
    @sachinmesh

@sachinmesh
Copy link

Looking good to me. Thank you @nimithshetty17

@keith-kaplan @anabarcelona

sachinmesh
sachinmesh suggested changes Nov 15, 2023
View reviewed changes
Copy link

@sachinmesh sachinmesh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  1. Need to remove the white divider line at the bottom of the iframe.
image
  1. Add links to the footer tabs and on hover add white underline.

@nimithshetty17 @keith-kaplan @anabarcelona

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 16, 2023

Page Scores Audits Google
/drafts/nimith/navattic-updated PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

@sachinmesh - Removed the white line below iframe.
@keith-kaplan we are good to merge the PR.

@sachinmesh
Copy link

Footer links are missing, rest is good to go @nimithshetty17 @keith-kaplan

@sachinmesh
Copy link

The footer links should not be in purple, on hover a white underline will come.
image

@nimithshetty17 @keith-kaplan @anabarcelona

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/nimith/navattic-updated PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

@sachinmesh - Footer issues have been addressed.
@keith-kaplan @anabarcelona

@sachinmesh sachinmesh added Ready to merge Label for the pull requests that are ready to merge and removed 🛠️ fix needed labels Nov 17, 2023
@keith-kaplan keith-kaplan merged commit 5ab1853 into hlxsites:main Nov 17, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anabarcelona anabarcelona Awaiting requested review from anabarcelona

@keith-kaplan keith-kaplan Awaiting requested review from keith-kaplan

@sachinmesh sachinmesh Awaiting requested review from sachinmesh

Assignees
No one assigned
Labels
Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nimithshetty17 @keith-kaplan @sachinmesh