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

fix: adjust search container width to align with page contents #1105

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

vpicone
Copy link
Contributor

@vpicone vpicone commented Mar 16, 2021

closes #1101

Adds some media queries to handle search width at various viewports.

@vpicone vpicone requested review from a team, aledavila and sstrubberg and removed request for a team March 16, 2021 22:24
@vercel
Copy link

vercel bot commented Mar 16, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-design-system/gatsby-theme-carbon/AdSk6Hoh5tLGBLKrBTMFKzie4E6g
✅ Preview: https://gatsby-theme-c-git-fork-vpicone-search-width-carbon-desi-58211c.vercel.app

@alisonjoseph
Copy link
Member

The calculations seem off by a bit on my 13 inch MacBook fullscreen. 🤔

Screen Shot 2021-03-16 at 8 56 08 PM

@alisonjoseph alisonjoseph requested a review from mjabbink March 19, 2021 14:12
@mjabbink
Copy link
Contributor

Preview looks good in both desktop and mobile

@vpicone
Copy link
Contributor Author

vpicone commented Mar 23, 2021

@jeanservaas made it so we only expand to 100% when the left nav is hidden. Once we get out of tablet range, it maxes out at the original spec/component implementation.

@mjabbink
Copy link
Contributor

Preview looks wrong now? What happened?

@mjabbink
Copy link
Contributor

MOBILE CORRECT
Screen Shot 2021-03-23 at 1 29 08 PM

@mjabbink
Copy link
Contributor

TABLET CORRECT
Screen Shot 2021-03-23 at 1 28 41 PM

@mjabbink
Copy link
Contributor

DESKTOP — Needs to be to one of the green lines. I believe the space was for the far left one.

Screen Shot 2021-03-23 at 1 30 58 PM

@mjabbink
Copy link
Contributor

Also seeing this in the preview for “Workplace Design” — https://ibm-brand.github.io/workplace-xg/

Screen Shot 2021-03-23 at 1 58 10 PM

@vpicone
Copy link
Contributor Author

vpicone commented Mar 24, 2021

@mjabbink @aagonzales and @jeanservaas okayed returning to the original search spec on desktop.

Because the shell sits above and separate from the page contents, justifying to that content requires hacky approximations at each viewport with unpleasant edge cases (see @alisonjoseph screenshot)

@vpicone
Copy link
Contributor Author

vpicone commented Mar 24, 2021

Reason why it requires hacks: the page container pushes from the left and the search emerges from the right.

We also need to subtract the switcher menu which gets spicy.

@vpicone
Copy link
Contributor Author

vpicone commented Mar 24, 2021

Also seeing this in the preview for “Workplace Design” — https://ibm-brand.github.io/workplace-xg/

Screen Shot 2021-03-23 at 1 58 10 PM

Its a theme issue so they'll get a fix once we ship this and they update.

Copy link
Member

@alisonjoseph alisonjoseph left a comment

Choose a reason for hiding this comment

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

LGTM!

@mjabbink
Copy link
Contributor

In the last image the search is spanning too far.

@mjabbink
Copy link
Contributor

Looks good to me too as long as the view deployment button above is what I’m supposed to look at.

@vpicone vpicone merged commit c08db04 into carbon-design-system:main Mar 24, 2021
@vpicone vpicone deleted the search-width branch March 24, 2021 20:07
@mjabbink
Copy link
Contributor

@vpicone @alisonjoseph This seems to have reverted because it’s the same as before the issue was made.

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.

Search field is too wide on desktop
3 participants