feat(header): L3-4326 updated spacing and search click #408

constantinehuzenko · 2024-10-29T17:29:31Z

Jira ticket

Screenshots

Before	After

Before	After

Summary

Remove margin from menu item, fixed logo margin, fixed auction links (need to update remix to see change), updated search click

Change List (describe the changes made to the files)

src/components/Navigation/NavigationItem/_navigationItem.scss: fixed mobile links
src/components/Navigation/NavigationList/_navigationList.scss: removed unnecessary spacing
src/components/Search/Search.tsx: added click on container for mobile view
src/site-furniture/Header/_header.scss: removed margin top for logo

Acceptance Test (how to verify the PR)

verify that auctions links are aligned on mobile
verify there is no bottom spacing after Editorial menu on mobile
verify that you can open search clicking on search container, not icon only (mobile)

Regression Test

(Optional) Add verification steps to make sure this PR doesn't break old functionality

Evidence of testing

Post logs, screenshots, etc

Things to look for during review

PR title should correctly describe the most significant type of commit. I.e. feat(scope): ... if a minor release should be triggered.
All commit messages follow convention and are appropriate for the changes
All references to phillips class prefix are using the prefix variable
All major areas have a data-testid attribute.
Document all props with jsdoc comments
All strings should be translatable.
Unit tests should be written and should have a coverage of 90% or higher in all areas.

New Components

Are there any accessibility considerations that need to be taken into account and tested?
Default story called "Playground" should be created for all new components
Create a jsdoc comment that has an Overview section and a link to the Figma design for the component
Export the component and its typescript type from the index.ts file
Import the component scss file into the componentStyles.scss file.

netlify · 2024-10-29T17:29:50Z

✅ Deploy Preview for phillips-seldon ready!

Name	Link
🔨 Latest commit	`7851874`
🔍 Latest deploy log	https://app.netlify.com/sites/phillips-seldon/deploys/67228c083ca5450008d6f830
😎 Deploy Preview	https://deploy-preview-408--phillips-seldon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

adietrich3074

Why are we removing the space between the language selector and the main nav list? I remember being explicitly told to add this in. Was the figma for the mobile nav updated?

constantinehuzenko · 2024-10-29T19:25:11Z

Why are we removing the space between the language selector and the main nav list? I remember being explicitly told to add this in. Was the figma for the mobile nav updated?

we don't have it on Prod, now I'm thinking if we should have it there

davidicus · 2024-10-29T21:08:54Z

src/components/Navigation/NavigationItem/_navigationItem.scss

@@ -69,6 +69,10 @@
    margin-bottom: 0;
  }

+  a {
+    display: inline;


This should probably be inline-block. Unless this is to shrink the clickable area

Looking on mobile, I think we may want to add a media query to only apply this to desktop sizes. I think on mobile the expectation would be to have the whole menu bar be clickable, no?

@constantinehuzenko Was this added explicitly for mobile? I think the expectation on mobile would be that the whole line would be the link and not jsut the texts, because of how we have styled it.

On desktop I would make it inline-block. Also the LI seems to be inline. That should be made to be inline-block too or removed all together .

davidicus · 2024-10-29T22:15:09Z

As Alex mentioned, there is spacing in between the language selector and the last link as shown in the figma https://www.figma.com/design/hMu9IWH5N3KamJy8tLFdyV/EASEL-Compendium%3A-Tokens%2C-Components-%26-Patterns?node-id=10572-6185&node-type=canvas&m=dev

constantinehuzenko · 2024-10-29T22:53:04Z

As Alex mentioned, there is spacing in between the language selector and the last link as shown in the figma https://www.figma.com/design/hMu9IWH5N3KamJy8tLFdyV/EASEL-Compendium%3A-Tokens%2C-Components-%26-Patterns?node-id=10572-6185&node-type=canvas&m=dev

Ryan approved that we shouldn't add space there

davidicus · 2024-10-30T20:32:48Z

src/components/Navigation/NavigationItem/_navigationItem.scss

@@ -69,6 +69,12 @@
    margin-bottom: 0;
  }

+  a {
+    @include isHeaderMobile {
+      display: block;


This take care of mobile but you still have the pointer on desktop when its not on the link. If we take the cursor pointer off the li it will just turn to pointer when on anchor.

# [1.86.0](v1.85.1...v1.86.0) (2024-10-30) ### Features * **header:** L3-4326 updated spacing and search click ([#408](#408)) ([01391e2](01391e2))

davidicus · 2024-10-30T20:55:57Z

🎉 This issue has been resolved in version 1.86.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

feat(header): L3-4326 updated spacing and search click

1af598a

constantinehuzenko requested review from davidicus, justinelliottcobb, adietrich3074, scottdickerson, aahill50 and sgrund14 as code owners October 29, 2024 17:29

feat(header): L3-4326 updated search logic and styles

03bfb3c

adietrich3074 reviewed Oct 29, 2024

View reviewed changes

adietrich3074 approved these changes Oct 29, 2024

View reviewed changes

davidicus reviewed Oct 29, 2024

View reviewed changes

feat(header): L3-4326 updated nav links display

abcfb04

constantinehuzenko added 2 commits October 30, 2024 12:06

feat(header): L3-4328 fixed view all link

fe4083f

feat(header): L3-4328 added tests

7851874

davidicus reviewed Oct 30, 2024

View reviewed changes

davidicus approved these changes Oct 30, 2024

View reviewed changes

constantinehuzenko merged commit 01391e2 into main Oct 30, 2024
14 checks passed

constantinehuzenko deleted the L3-4326-header-updates branch October 30, 2024 20:54

davidicus pushed a commit that referenced this pull request Oct 30, 2024

chore(release): 1.86.0 [skip ci]

9f61b12

# [1.86.0](v1.85.1...v1.86.0) (2024-10-30) ### Features * **header:** L3-4326 updated spacing and search click ([#408](#408)) ([01391e2](01391e2))

davidicus added the released label Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(header): L3-4326 updated spacing and search click #408

feat(header): L3-4326 updated spacing and search click #408

constantinehuzenko commented Oct 29, 2024 •

edited by jira bot

Loading

netlify bot commented Oct 29, 2024 •

edited

Loading

adietrich3074 left a comment

constantinehuzenko commented Oct 29, 2024

davidicus Oct 29, 2024

davidicus Oct 29, 2024

davidicus Oct 29, 2024

davidicus commented Oct 29, 2024

constantinehuzenko commented Oct 29, 2024

davidicus Oct 30, 2024

davidicus commented Oct 30, 2024

feat(header): L3-4326 updated spacing and search click #408

feat(header): L3-4326 updated spacing and search click #408

Conversation

constantinehuzenko commented Oct 29, 2024 • edited by jira bot Loading

netlify bot commented Oct 29, 2024 • edited Loading

✅ Deploy Preview for phillips-seldon ready!

adietrich3074 left a comment

Choose a reason for hiding this comment

constantinehuzenko commented Oct 29, 2024

davidicus Oct 29, 2024

Choose a reason for hiding this comment

davidicus Oct 29, 2024

Choose a reason for hiding this comment

davidicus Oct 29, 2024

Choose a reason for hiding this comment

davidicus commented Oct 29, 2024

constantinehuzenko commented Oct 29, 2024

davidicus Oct 30, 2024

Choose a reason for hiding this comment

davidicus commented Oct 30, 2024

constantinehuzenko commented Oct 29, 2024 •

edited by jira bot

Loading

netlify bot commented Oct 29, 2024 •

edited

Loading