feat: implement account list item component #17303
Merged
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
briansztamfater
changed the title
Jenkins BuildsClick to see older builds (73)
|
briansztamfater
force-pushed
the
feat/account-list-item
branch
7 times, most recently
from
c2f7d84 to
0f6294b
Compare
briansztamfater
changed the title
briansztamfater
requested review from
FFFra,
tumanov-alex,
smohamedjavid,
J-Son89,
mmilad75 and
mohsen-ghafouri
briansztamfater
force-pushed
the
feat/account-list-item
branch
from
0f6294b to
c7e3aa4
Compare
briansztamfater
force-pushed
the
feat/account-list-item
branch
2 times, most recently
from
4a7792d to
a934ff7
Compare
clauxx
requested changes
Sep 25, 2023
Comment on lines
14
to
22
| (h/wait-for #(h/has-style (h/query-by-label-text :container) | ||
| {:height 56 | ||
| :borderRadius 12 | ||
| :backgroundColor (colors/custom-color :blue 50 5) | ||
| :flexDirection :row | ||
| :alignItems :center | ||
| :paddingHorizontal 12 | ||
| :paddingVertical 6 | ||
| :justifyContent :space-between}))) |
There was a problem hiding this comment.
Not sure if we get a value from this kind of strict-styling tests, given that styles might be changing and it would add extra work. Could instead use snapshot testing (mehh) which would have a similar result (but easier to maintain), or just do the check for the background color, since it seems like it's the only style that changes.
There was a problem hiding this comment.
Agree, see my comment in this other PR -> #17398 (comment)
I'll update on this once I have more inputs either on this or the other PR
There was a problem hiding this comment.
For the beginning I would just strip the tests of all the unnecessary styles and just keep the style u test for (here it's backgroundColor). For me it would be enough to have it merged, and then decide on the usefulness of these tests in a follow-up. We already have quite a few tests that check for styles in this way, so I guess it should be fine.
There was a problem hiding this comment.
Nice, I thought toHaveStyle checked for the entire style map, but it works also checking for a specific style prop, so I simplified it as suggested :)
briansztamfater
force-pushed
the
feat/account-list-item
branch
2 times, most recently
from
53cd45a to
cdc4b05
Compare
vkjr
approved these changes
Sep 26, 2023
briansztamfater
force-pushed
the
feat/account-list-item
branch
2 times, most recently
from
ad741d3 to
8194150
Compare
|
@Francesca-G can you review this component please? |
clauxx
approved these changes
Sep 27, 2023
Francesca-G
approved these changes
Sep 27, 2023
There was a problem hiding this comment.
Looks good ✨
Adding follow up required to fix the spacing between 1st and 2nd line, see comment here :)
briansztamfater
force-pushed
the
feat/account-list-item
branch
from
082553b to
3fb70b5
Compare
|
@Francesca-G I adjusted spacing in balance text at the right, is that ok? |
briansztamfater
force-pushed
the
feat/account-list-item
branch
from
3fb70b5 to
774775b
Compare
Signed-off-by: Brian Sztamfater <brian@status.im>
briansztamfater
force-pushed
the
feat/account-list-item
branch
from
774775b to
0ba010e
Compare
ibrkhalil
pushed a commit
that referenced
this pull request
Oct 1, 2023
Signed-off-by: Brian Sztamfater <brian@status.im>
clauxx
pushed a commit
that referenced
this pull request
Oct 3, 2023
Signed-off-by: Brian Sztamfater <brian@status.im>
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 #17301
Summary
Implement Account List Item component
Screenshots
Platforms
Steps to test
status: ready