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: Fix Desktop rhombus bug on Safari #188

Merged
merged 7 commits into from
Jun 14, 2023

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Jun 14, 2023

closes #174 fully

How to test

  • Apple/Mac OS: test on Safari, Firefox, Chrome for rhombus fixes
  • Windows: test on Firefox, Chrome for no regressions
  • Android: test on Chrome for no regressions
  • iPhone: test on Safari for no regressions

Before & After

image image Safari behaves very strangely with `filter: url()` -- the browser is trying to apply the filter to _everything_, from the link hover effect, to even the way the user highlights the text. There is no way to apply the filter to just one element in Safari, the browser will apply it to every child element and even highlighted text. And there's no way to add another filter to undo a url filter.
Before After
image image
Totally bonkers. Normal.
image image

@netlify
Copy link

netlify bot commented Jun 14, 2023

Deploy Preview for cal-itp-website ready!

Name Link
🔨 Latest commit d98fdd7
🔍 Latest deploy log https://app.netlify.com/sites/cal-itp-website/deploys/6489fc54a8d95600088516e7
😎 Deploy Preview https://deploy-preview-188--cal-itp-website.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@machikoyasuda machikoyasuda marked this pull request as ready for review June 14, 2023 03:54
@machikoyasuda machikoyasuda requested a review from a team as a code owner June 14, 2023 03:54
@machikoyasuda machikoyasuda self-assigned this Jun 14, 2023
@machikoyasuda machikoyasuda added this to the Calitp.org redesign milestone Jun 14, 2023
@thekaveman
Copy link
Member

@segacy1 when you have some time, can you please review this follow-up to the split rhombuses in Mac/Safari -- this time for Desktop widths. (And please ensure no regressions in Mobile as well).

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Regressions on homepage Contact rhombuses, in Windows/Desktop Firefox and Chrome:

  • Bottoms are no longer aligned (seems to only happen in "Tablet" sizes, e.g. less than full-screen but larger than mobile)
  • The direction changed and no longer represents the Figma design

Figma

image

Staging

Bottom of each rhombus is aligned, direction "towards" each other as shown in Figma:

image

This PR

Bottom of the left rhombus is lower than the bottom of the right rhombus, direction "away" from each other:

image

@machikoyasuda
Copy link
Member Author

Fixes pushed:

  • Switches the shapes around
  • Uses flex-box to manage / keep the heights the same
  • Adds back shadow

Safari:
image
image

Firefox:
image
image

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

Looks great! Confirmed the fix in Desktop Firefox/Chrome on Windows.

Also, thank you for removing that inline style definition 😁

@machikoyasuda machikoyasuda merged commit 9984e09 into staging Jun 14, 2023
6 checks passed
@machikoyasuda machikoyasuda deleted the fix/desktop-rhombus-filter-bug branch June 14, 2023 21:25
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.

Investigate split rectangle on Press/Resources in Mobile
3 participants