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

[IOAPPX-318] Add contrast variant and optional backgroundColor prop to HeaderSecondLevel #285

Merged
merged 4 commits into from
Jun 12, 2024

Conversation

dmnplb
Copy link
Collaborator

@dmnplb dmnplb commented Jun 10, 2024

Short description

This PR adds a contrast variant (light icons on dark background) and an optional backgroundColor prop to set a custom background color when there's a transition between the transparent → solid state.

List of changes proposed in this pull request

  • Add variant and backgroundColor props

Preview

header2ndLevel-customBG.mp4

How to test

  1. Launch the example app
  2. Check every example with HeaderSecondLevel, including the new custom background

@dmnplb dmnplb added the enhancement New feature or request label Jun 10, 2024
@dmnplb dmnplb requested review from a team as code owners June 10, 2024 12:42
Copy link
Contributor

@hevelius hevelius left a comment

Choose a reason for hiding this comment

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

LGTM. Tested and works as expected. @mastro993 having already encountered this case, what do you think?

@mastro993
Copy link
Contributor

I don't know if this is expected or not (maybe is just the example), but on Android I see a white header.

Registrazione.schermo.2024-06-12.alle.15.00.06.mov

@dmnplb
Copy link
Collaborator Author

dmnplb commented Jun 12, 2024

@mastro993 Considering that the blue block is just an ugly block with position: absolute and a negative top value to quickly test a dark background, it's probably limited to the example. But let me check 👍🏻

@dmnplb
Copy link
Collaborator Author

dmnplb commented Jun 12, 2024

@mastro993 Just fixed the example screen on both iOS and Android ✌🏻

Copy link
Contributor

@mastro993 mastro993 left a comment

Choose a reason for hiding this comment

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

LGTM! Looks good on both Android and iOS!

@dmnplb dmnplb merged commit d296b5a into main Jun 12, 2024
6 checks passed
@dmnplb dmnplb deleted the IOAPPX-318-add-contrast-variant-header-second-level branch June 12, 2024 18:05
dmnplb added a commit that referenced this pull request Jun 18, 2024
## Short description
This PR fixes the `HeaderSecondLevel` UI regression, introduced by
mistake with the following PR:
* #285
The problem 

## List of changes proposed in this pull request
- Apply the `animatedStyle` in all the conditions

### Preview
(Please look at the header's bottom border)
| Before | After |
|--------|--------|
| <video
src="https://github.com/pagopa/io-app-design-system/assets/1255491/cafdf95b-b4a4-4b5c-a316-21708a04c758"
/> | <video
src="https://github.com/pagopa/io-app-design-system/assets/1255491/96dc7c95-e658-4f12-95ff-e4b9efed3565"
/> |

## How to test
1. Launch the example app
2. Check out every example that's using `HeaderSecondLevel`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants