hlxsites / delta

https://main--delta--hlxsites.hlx.page/us/en/skymiles/overview
Apache License 2.0
2 stars 3 forks source link

feat: implement section navigation #93

Closed ramboz closed 1 year ago

ramboz commented 1 year ago

Adds the section navigation dropdown menu just below the header.

Fix #92

Test URLs:

aem-code-sync[bot] commented 1 year ago

Hello, I'm Franklin 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.

aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
ramboz commented 1 year ago
Screenshot 2023-03-14 at 1 38 04 PM Screenshot 2023-03-14 at 1 37 58 PM Screenshot 2023-03-14 at 1 37 45 PM Screenshot 2023-03-14 at 1 37 29 PM
aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
asthabh23 commented 1 year ago

@ramboz , few observations from my side which would require a fix:

  1. On clicking an item in the submenu list, the other open item should collapse. Current franklin site (here both submenu options are visible at the same time): Screenshot 2023-03-15 at 1 26 21 PM

Original site (when one is visible, others should collapse):

Screenshot 2023-03-15 at 1 27 25 PM
  1. The red up and down arrows seem to be a bit bigger in size compared to original site. Also, they are not visible between 768px and 1200px. Current site (at 1000px): Screenshot 2023-03-15 at 1 29 20 PM

Original site (at 1000px):

Screenshot 2023-03-15 at 1 29 53 PM
  1. Below 768px, the menu seems to have some margin around it on original site, which is missing in our implementation (we can keep the current version too, still just a point in case required to be modified): Current franklin site: Screenshot 2023-03-15 at 1 32 26 PM

original Delta site:

Screenshot 2023-03-15 at 1 34 46 PM

**Inputs/suggestions on some improvements we already have in place** The original site section navigation seems to be a bit off:

ramboz commented 1 year ago

@asthabh23 Thanks for the thorough review! I agree on all points and have addressed those in the latest commit. I also added a bit of animation like on the official website.

aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
asthabh23 commented 1 year ago

@ramboz , thanks for the fixes you have added. Few observation points:

  1. When I hover/focus over the section navigation menu, I see a layout shift that happens (the downward arrow next to "GET TO KNOW SKYMILES" along with "How to Earn miles" - these two together shift towards the right). These needs a fix

  2. There is a slight gap between the section and the menu that pops up on clicking "GET TO KNOW MILES" Reference screenshot:

    Screenshot 2023-03-16 at 11 47 06 AM
  3. On original site, when I click on "GET TO KNOW MILES", there is a border that comes up, possibly we can add that as well.

    Screenshot 2023-03-16 at 12 05 32 PM
aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI
ramboz commented 1 year ago

@asthabh23 I think I addressed the 3 points… if I understood those correctly. Let me know what you think!

ramboz commented 1 year ago

Yeah good point on the header leaking into the new styles

aem-code-sync[bot] commented 1 year ago
Page Score PSI Audit Google
/us/en/skymiles/how-to-earn-miles/overview SI FCP LCP TBT CLS PSI