Quo2: Account list card blur #17271
Merged
Quo2: Account list card blur #17271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (8)
|
smohamedjavid
approved these changes
Sep 13, 2023
Comment on lines
23
to
24
| {:style {:flex-direction :row | ||
| :align-items :center}} |
There was a problem hiding this comment.
We can move these styles to style.cljs.
J-Son89
reviewed
Sep 13, 2023
| @@ -2,16 +2,16 @@ | |||
| (:require [quo2.foundations.colors :as colors])) | |||
|
|
|||
| (defn container | |||
| [state theme] | |||
| [state blur? theme] | |||
There was a problem hiding this comment.
consider making this a map 👍
J-Son89
approved these changes
Sep 13, 2023
14% of end-end tests have passedFailed tests (37)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
@Francesca-G a design review :) |
Francesca-G
approved these changes
Sep 13, 2023
There was a problem hiding this comment.
Component looks good ✨
@OmarBasem I've noticed a bug, if I select the blur theme it automatically goes to dark theme (which is correct), but if I try to press the theme switch button at the top when I'm In this state it goes back to light theme with blur: true, which shouldn't happen
|
Imo it would be nicer if we could just disable the blur toggle in light mode rather than swapping the theme etc also while all these new updates to preview screens are welcome and very appreciated! I think we would all benefit from having some quick discussion among 1 or 2 devs to agree upon the new idea/approach. Although perhaps that already happened in this case 🤔 |
7% of end-end tests have passedFailed tests (40)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Passed tests (3)Click to expandClass TestCommunityOneDeviceMerged:
|
ilmotta
approved these changes
Sep 13, 2023
I certainly agree with you @J-Son89. A bit off-topic, but to each their preference, whatever we decide to do, my vote will probably go to the solution that keeps previews dumb and flat so that we can completely auto-generate 90%+ of them (esp. if we get Malli in). |
Sound good, we can do that |
9% of end-end tests have passedFailed tests (39)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (4)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
|
briansztamfater
approved these changes
Sep 13, 2023
@Francesca-G thanks for spotting this! |
OmarBasem
force-pushed
the
quo2/account-list-card-blur
branch
from
983e6ea
to
177c870
Compare
12% of end-end tests have passedFailed tests (38)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (5)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
@churik e2e seems to be failing. Does it have to do anything with this PR? |
|
e2e are not relevant now because of #17196 |
|
Merging of all PRs is blocked until we find a reason |
OmarBasem
force-pushed
the
quo2/account-list-card-blur
branch
from
177c870
to
a9f2ccb
Compare
|
@OmarBasem ready to merge if this component is not used anywhere in the app |
23% of end-end tests have passedFailed tests (33)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (10)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
OmarBasem
force-pushed
the
quo2/account-list-card-blur
branch
from
a9f2ccb
to
a6da2ca
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes: #17272
This PR implements blur for the account list card component.
Designs