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: ios button card style issue #2302

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

jfmcquade
Copy link
Collaborator

@jfmcquade jfmcquade commented Apr 25, 2024

PR Checklist

  • PR title descriptive (can be used in release notes)

Description

Fixes an issue where the background of the button component with variant: card applied would turn transparent when scrolled off screen and back again.

This is a frustratingly simple solution to an issue that cause some headaches: the card variant of the button component had an extra drop-shadow filter applied, in addition to the box-shadow applied to the ionic button component. Removing this additional drop-shadow fixes the issue.

Dev notes

Before I removed this filter, the behaviour was very strange (I've added some brief notes to #2287 in case they're useful in future). I think there is possibly a genuine bug that could be reported to ionic (or even Apple), along the lines of "Setting both a CSS drop-shadow filter and a box-shadow on an element causes CSS variables to not be passed correctly to children", but I don't think I've managed to isolate the bug fully. And this is a pattern we can easily avoid.

Git Issues

Closes #2287

Screenshots/Videos

I've made a new template, debug_card_styles, for visually comparing all the components that have are styled as a "card". The fact that we have different components that look so similar is confusing, but I think still justified as they are functionally fundamentally different (a task-card necessarily has an associated task or task group, which has a completion status, whereas a button has no such association). Comparing these elements before and after the changes made by this PR, we can see that in the "before", the button with variant: card stands out as having a darker shadow than the other elements. In the "after" the drop-shadows for all elements match. I don't think there is any reason for this other than an error made when duplicating some CSS code (the card-portrait variant of the button does not use an ion-button as its base, so deliberately has this filter applied).

Before After
Before After

Working demo

Demo of comp_button component showing bug not present (see this comment on the issue for previous behaviour).

button.fixed.mov

Copy link

github-actions bot commented Apr 25, 2024

Visual Test Summary
new : 0
different : 6
same : 344

Largest Differences
1 | 3.8 % | comp_button
2 | 1.2 % | debug_card_styles
3 | 0.4 % | user_actions
4 | 0.1 % | example_calc_date
5 | 0.1 % | example_calc
6 | 0 % | feature_parent_point_box

Download Link
https://nightly.link/IDEMSInternational/parenting-app-ui/actions/runs/8982315093

Run Details
https://github.com/IDEMSInternational/parenting-app-ui/actions/runs/8982315093

@github-actions github-actions bot added fix and removed fix labels Apr 25, 2024
@github-actions github-actions bot added fix and removed fix labels Apr 25, 2024
Copy link
Member

@chrismclarke chrismclarke left a comment

Choose a reason for hiding this comment

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

Nice catch! And yeah super frustrating indeed, particularly given how different browsers handle these differently

@github-actions github-actions bot added fix and removed fix labels May 7, 2024
@esmeetewinkel esmeetewinkel merged commit a8ba99e into master May 7, 2024
8 checks passed
@esmeetewinkel esmeetewinkel deleted the fix/ios-button-card-style branch May 7, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix test - visual Run visual regression tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] iOS button shadow
3 participants