Skip to content

feat: Reduce blurriness of container shadows against dark header background in VR#702

Merged
jkuelz merged 2 commits intomainfrom
jkuelz-vr-shadows-rebased-pr
Feb 7, 2023
Merged

feat: Reduce blurriness of container shadows against dark header background in VR#702
jkuelz merged 2 commits intomainfrom
jkuelz-vr-shadows-rebased-pr

Conversation

@jkuelz
Copy link
Member

@jkuelz jkuelz commented Feb 3, 2023

Description

The main purpose of this commit is to reduce the "blurriness" or "halo" effect when a container shadow overlaps a dark header in VR (AWSUI-19630). See screenshot below for before/after (note: the previous is on the right, the updated is on the left):

github-vr-shadows-light-screenshot

The way it accomplishes this is by adding mix-blend-mode: multiply to container shadows. In order to ensure the mix-blend-mode only effects the container shadow and not the content inside, the shadow has been moved to a pseudo element (::after) so that it can be treated as it’s own layer.

With this change, the container border has also been moved to a separate pseudo element (::before), so it can receive a different stacking context from the shadow and not blend with the background. This largely removes the need to account for the border in sticky header offset calculations, as well as padding values in selected cards.

As a result, this commit also removes some outdated code that was originally needed for IE11 support, as well as some border calculations that are no longer needed.

Lastly, this change slightly increases the contrast of VR shadows in dark mode to make the container perimeter more noticeable, which has also been repeated feedback (AWSUI-16528). The updates have been reviewed and approved by visual design. Note: the previous is on the right, the updated is on the left:

Screen Shot 2023-02-02 at 11 59 37 PM

Related links, issue #, if available: AWSUI-19630, AWSUI-16528

How has this been tested?

Ran through the pipeline; checked screenshots in Chrome, Firefox, Safari. Also reviewed with visual designer to approve shadow changes.

See changes in pipeline: AwsUi-v3-jkuelz (approval attempt: 314060285, 314765186)

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jkuelz jkuelz requested a review from a team as a code owner February 3, 2023 09:01
@jkuelz jkuelz requested review from abdhalees and removed request for a team February 3, 2023 09:01
@michaeldowseza michaeldowseza requested review from michaeldowseza and removed request for abdhalees February 3, 2023 09:06
@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Base: 93.27% // Head: 93.26% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (85155a9) compared to base (112a9cb).
Patch coverage: 75.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #702      +/-   ##
==========================================
- Coverage   93.27%   93.26%   -0.01%     
==========================================
  Files         584      584              
  Lines       17075    17066       -9     
  Branches     4704     4699       -5     
==========================================
- Hits        15926    15917       -9     
  Misses       1069     1069              
  Partials       80       80              
Impacted Files Coverage Δ
src/container/use-sticky-header.ts 79.48% <50.00%> (-4.19%) ⬇️
src/container/internal.tsx 95.83% <100.00%> (+0.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@michaeldowseza michaeldowseza left a comment

Choose a reason for hiding this comment

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

Added some comments . A lot of the consts now change to a 0px offset and can probably be removed.

@jkuelz jkuelz force-pushed the jkuelz-vr-shadows-rebased-pr branch 2 times, most recently from 1374b9c to bfb3395 Compare February 6, 2023 22:00
@jkuelz jkuelz force-pushed the jkuelz-vr-shadows-rebased-pr branch from bfb3395 to 85155a9 Compare February 7, 2023 15:49
@jkuelz jkuelz merged commit 222b99f into main Feb 7, 2023
@jkuelz jkuelz deleted the jkuelz-vr-shadows-rebased-pr branch February 7, 2023 19:33
just-boris added a commit that referenced this pull request Feb 8, 2023
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.

2 participants