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

(PC-21416) feat(statusBar): blur/white status bar home and profile #5089

Merged
merged 4 commits into from
Aug 1, 2023

Conversation

S4andro
Copy link
Contributor

@S4andro S4andro commented Jul 28, 2023

Link to JIRA ticket: https://passculture.atlassian.net/browse/PC-21416

Checklist

I have:

  • Made sure my feature is working on the relevant real / virtual devices (native and web).
  • Written unit tests native (and web when implementation is different) for my feature.
  • Added a screenshot for UI tickets or deleted the screenshot section if no UI change
  • If my PR is a bugfix, I add the link of the "résolution de problème sur le bug" [on Notion][1]

Screenshots

Platform Before After
iOS Capture d’écran 2023-07-28 à 13 58 27 Capture d’écran 2023-07-28 à 13 58 00
Android Capture d’écran 2023-07-28 à 13 58 45 Capture d’écran 2023-07-28 à 13 59 02

@S4andro S4andro force-pushed the PC-21416-blur-status-bar-home-and-profile branch 2 times, most recently from c8719ea to b39bcf9 Compare July 28, 2023 12:05
<Section title={isLoggedIn ? 'Paramètres du compte' : 'Paramètres de l’application'}>
<VerticalUl>
{!!isLoggedIn && (
<View>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dans ce fichier j'ai juste rajouter en ligne 117, 263, 264

@github-actions
Copy link

github-actions bot commented Jul 28, 2023

Performance Comparison Report

Significant Changes To Render Duration

There are no entries

Meaningless Changes To Render Duration

Show entries
Name Render Duration Render Count
Performance test for Offer page 650.5 ms → 661.0 ms (+10.5 ms, +1.6%) 10 → 10
Search Results - Performance test for Search Results page 146.5 ms → 151.1 ms (+4.6 ms, +3.1%) 9.80 → 10 (+0.1999999999999993, +2.0%)
Performance test for Profile page 174.1 ms → 176.9 ms (+2.8 ms, +1.6%) 7 → 7
Search Suggestions - Performance test for Search Suggestions page 50.6 ms → 50.9 ms (+0.3 ms, +0.6%) 6 → 6
Performance test for Bookings page 55.0 ms → 52.4 ms (-2.6 ms, -4.7%) 6 → 6
Performance test for EndedBookings page 35.2 ms → 32.3 ms (-2.9 ms, -8.2%) 6 → 6
Search Landing Page - Performance test for Search Landing page 118.2 ms → 115.1 ms (-3.1 ms, -2.6%) 6 → 6
Performance test for Favorites page 174.7 ms → 168.3 ms (-6.4 ms, -3.7%) 7 → 7
Performance test for Venue page 582.3 ms → 564.6 ms (-17.7 ms, -3.0%) 9 → 9
Show details
Name Render Duration Render Count
Performance test for Offer page Baseline
Mean: 650.5 ms
Stdev: 19.9 ms (3.1%)
Runs: 675 672 670 669 654 645 633 633 628 626

Current
Mean: 661.0 ms
Stdev: 24.2 ms (3.7%)
Runs: 701 699 671 669 657 655 645 644 639 630
Baseline
Mean: 10
Stdev: 0 (0.0%)
Runs: 10 10 10 10 10 10 10 10 10 10

Current
Mean: 10
Stdev: 0 (0.0%)
Runs: 10 10 10 10 10 10 10 10 10 10
Search Results - Performance test for Search Results page Baseline
Mean: 146.5 ms
Stdev: 10.3 ms (7.1%)
Runs: 166 157 152 150 148 146 140 138 134 134

Current
Mean: 151.1 ms
Stdev: 13.4 ms (8.9%)
Runs: 177 169 153 152 151 151 146 140 139 133
Baseline
Mean: 9.80
Stdev: 0.63 (6.5%)
Runs: 8 10 10 10 10 10 10 10 10 10

Current
Mean: 10
Stdev: 0 (0.0%)
Runs: 10 10 10 10 10 10 10 10 10 10
Performance test for Profile page Baseline
Mean: 174.1 ms
Stdev: 12.9 ms (7.4%)
Runs: 195 189 186 183 169 167 166 166 161 159

Current
Mean: 176.9 ms
Stdev: 12.0 ms (6.8%)
Runs: 199 196 181 175 173 173 172 172 167 161
Baseline
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7

Current
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7
Search Suggestions - Performance test for Search Suggestions page Baseline
Mean: 50.6 ms
Stdev: 5.0 ms (9.9%)
Runs: 60 55 55 51 50 49 48 48 48 42

Current
Mean: 50.9 ms
Stdev: 3.1 ms (6.2%)
Runs: 56 55 54 51 51 50 49 48 48 47
Baseline
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6

Current
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6
Performance test for Bookings page Baseline
Mean: 55.0 ms
Stdev: 5.7 ms (10.3%)
Runs: 68 58 57 57 55 53 53 51 51 47

Current
Mean: 52.4 ms
Stdev: 3.1 ms (5.8%)
Runs: 57 56 56 53 53 51 50 50 49 49
Baseline
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6

Current
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6
Performance test for EndedBookings page Baseline
Mean: 35.2 ms
Stdev: 4.7 ms (13.2%)
Runs: 45 39 37 37 36 34 32 32 31 29

Current
Mean: 32.3 ms
Stdev: 1.8 ms (5.7%)
Runs: 35 34 34 33 33 32 32 30 30 30
Baseline
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6

Current
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6
Search Landing Page - Performance test for Search Landing page Baseline
Mean: 118.2 ms
Stdev: 11.7 ms (9.9%)
Runs: 146 127 123 116 116 114 113 112 112 103

Current
Mean: 115.1 ms
Stdev: 3.7 ms (3.2%)
Runs: 119 119 119 118 116 115 112 112 112 109
Baseline
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6

Current
Mean: 6
Stdev: 0 (0.0%)
Runs: 6 6 6 6 6 6 6 6 6 6
Performance test for Favorites page Baseline
Mean: 174.7 ms
Stdev: 11.5 ms (6.6%)
Runs: 196 185 183 177 174 174 172 165 165 156

Current
Mean: 168.3 ms
Stdev: 11.7 ms (6.9%)
Runs: 189 180 179 170 170 164 163 161 154 153
Baseline
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7

Current
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7
Performance test for Venue page Baseline
Mean: 582.3 ms
Stdev: 35.3 ms (6.1%)
Runs: 659 620 595 581 574 571 568 563 559 533

Current
Mean: 564.6 ms
Stdev: 30.8 ms (5.5%)
Runs: 626 606 576 566 557 556 552 544 535 528
Baseline
Mean: 9
Stdev: 0 (0.0%)
Runs: 9 9 9 9 9 9 9 9 9 9

Current
Mean: 9
Stdev: 0 (0.0%)
Runs: 9 9 9 9 9 9 9 9 9 9

Changes To Render Count

There are no entries

Added Scenarios

There are no entries

Removed Scenarios

There are no entries

Generated by 🚫 dangerJS against 70789d5

export const StatusBarBlurredBackground = ({ blurAmount = 8, blurType = 'light' }: Props) => {
const { top } = useCustomSafeInsets()

// There is an issue with the blur on Android: we chose not to render it and use a white background
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce que tu peux ajouter le lien de l'issue github stp ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

j'ai juste copier coller le commentaire de BlurView, j'ai plus le lien exact de l'issue :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quand je dis lien exact, c'est que y en a plusieurs avec des problèmes liés à Android mais je retrouve pas celle qui le dit explicitement. Est ce que je mets un lien d'une de ces issues ?

Copy link
Contributor

Choose a reason for hiding this comment

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

En regardant rapidement j'ai l'impression que celle ci parle du souci qu'on a rencontré non ? Kureev/react-native-blur#511

Copy link
Contributor

Choose a reason for hiding this comment

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

Dans tous les cas oui ça peut être pas mal de mettre un lien

blurType?: 'dark' | 'light' | 'xlight'
}

export const StatusBarBlurredBackground = ({ blurAmount = 8, blurType = 'light' }: Props) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

D'après le code le composant BlurView a déjà ces propriétés :

export const BlurView = ({ blurAmount = 8, blurType = 'light' }: Props) => {
  return <Blurred blurType={blurType} blurAmount={blurAmount} />
}

Est-ce vraiment nécessaire de les remettre ici ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pas forcément, je les enlèvent

@S4andro S4andro force-pushed the PC-21416-blur-status-bar-home-and-profile branch from b39bcf9 to 0d6dfa3 Compare July 31, 2023 13:29
@S4andro S4andro force-pushed the PC-21416-blur-status-bar-home-and-profile branch from 0d6dfa3 to 629e27f Compare July 31, 2023 13:35
export const StatusBarBlurredBackground = ({ blurAmount = 8, blurType = 'light' }: Props) => {
const { top } = useCustomSafeInsets()

// There is an issue with the blur on Android: we chose not to render it and use a white background
Copy link
Contributor

Choose a reason for hiding this comment

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

En regardant rapidement j'ai l'impression que celle ci parle du souci qu'on a rencontré non ? Kureev/react-native-blur#511

export const StatusBarBlurredBackground = ({ blurAmount = 8, blurType = 'light' }: Props) => {
const { top } = useCustomSafeInsets()

// There is an issue with the blur on Android: we chose not to render it and use a white background
Copy link
Contributor

Choose a reason for hiding this comment

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

Dans tous les cas oui ça peut être pas mal de mettre un lien

@S4andro S4andro merged commit b5db6b2 into master Aug 1, 2023
@S4andro S4andro deleted the PC-21416-blur-status-bar-home-and-profile branch August 1, 2023 07:30
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.

3 participants