feat: category quo2 component #16713
Conversation
OmarBasem
requested review from
briansztamfater,
smohamedjavid,
J-Son89 and
ulisesmac
|
any chance we can add a simple component spec test here :) |
Jenkins BuildsClick to see older builds (16)
|
|
Hi @Francesca-G, can I get a design review, please? The component can be found in the |
There was a problem hiding this comment.
@OmarBasem here's the Figma frame with the design review for the Category component.
I've also noticed that the blur style is missing, but maybe you're planning on implementing it further on. Hope this helps!
|
Whoops @OmarBasem, @Francesca-G that was my fault about blue type as the issue says to leave it out - that was to keep the scope smaller as the issue was originally being done by an interview candidate. If possible can you include @OmarBasem 🙏 |
Oh sure I will update it now! |
53% of end-end tests have passedFailed tests (17)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (19)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
There was a problem hiding this comment.
Hello @OmarBasem
I switched to your branch, but unfortunately I couldn't test all the variants as in designs. We are usually creating preview screens that allow to customize component's parameters.
Could you please update the preview screen to make the component customizable?
i.e., being able to set "Label" to any text, change the list size, add icons/images at the right and left of the items in list to look as the designs, etc.
Thanks!
|
Hi @ulisesmac, Thanks for reviewing the PR. Sure I can add some customizations to update the text label and number of items. I don't see in the designs more customizations to be added. The component that has icons/images is another component that has another issue #16712 Just a small note, please use the "request changes" with care as it is not very flexible, especially with our very different timezones. If you see something catastrophic then sure use it :) Thank you! |
OK! nice, please add them :)
Yes, I know @OmarBasem, since this PR had been already approved, I just wanted to make sure this is fixed before merging :) |
Thanks for your review @Francesca-G ! |
|
|
||
| (defn items | ||
| [theme blur?] | ||
| {:margin-top 12 | ||
| :border-radius 16 | ||
| :background-color (if blur? | ||
| colors/white-opa-5 | ||
| (colors/theme-colors colors/white colors/neutral-95 theme)) | ||
| :border-width 1 | ||
| :border-color (if blur? | ||
| colors/white-opa-5 | ||
| (colors/theme-colors colors/neutral-10 colors/neutral-80 theme))}) | ||
|
|
There was a problem hiding this comment.
@J-Son89 out of scope for this PR, but I think we need to update the colors/theme-colors method to accept an optional third color as light, dark and blur as some parts of the app can have 3 themes like settings
There was a problem hiding this comment.
I was discussing this with @ulisesmac before, he was against the idea. Do you still have the same opinion @ulisesmac ?
There was a problem hiding this comment.
also it's 4 - light, dark light with blur, dark with blur.
There was a problem hiding this comment.
@J-Son89 I still think it might be a little confusing, but I would accept it now.
So, go ahead if you think is a good approach 👍
|
@OmarBasem please let us know when review comments will be resolved and and PR will be ready for manual QA. Thanx! |
|
Hi @pavloburykh, this is just a quo2 component. |
|
@ulisesmac PR is awaiting your review |
Got it! I thought there was a label Request manual QA. But now I see there is no. |
fixes: #16211
Summary
This PR implements the settings category component. Opened a separate issue for reorder component: #16712
Designs.
Preview