-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat: empty ui for create flows #42413
feat: empty ui for create flows #42413
Conversation
@rayane-djouah Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
From a copy perspective, it looks like we have an orphan in "number" - cc @jamesdeanexpensify Thoughts on forcing a line break before "or" so the bottom line reads "or phone number" ? |
@shawnborton tagged ya here to get alignment. |
@shawnborton @dannymcclain I updated screenshots after resolving feedbacks. Also, this is what it looks like when keyboard is open and I don't think it looks good. What do you think it should be like? |
@danielrvidal we're going with the updated copy below as decided here, right?
|
My take is that the illustration and text should ignore the keyboard and be centered vertically on the. That means that when the keyboard moves in it'll obscure parts of the text and illustration. But keen to hear what @Expensify/design thinks |
Yup, I agree with @dubielzyk-expensify in that the keyboard would sit on top of the content in this case. Also, I think we decided to only show the referral banner once you start seeing results in the contact list after searching? This way we aren't duplicating the same message from the empty state. |
Yup, I agree too. |
Just double checking - where should I confirm that we're updating to this copy? Thank you! |
@tienifr can you please make sure to update the copy and screenshots to reflect this comment from @jamesdeanexpensify. That matches the copy here, where you can also find the Spanish copy as well. |
Lint failing @tienifr |
…pty-ui-for-create-flows
I'm updating the copies and handle the keyboard overlapping behavior. Will finish today. |
…pty-ui-for-create-flows
@shawnborton @dubielzyk-expensify Let's check the following the result. Hopefully it's our expectation web-resize.mp4 |
Looooovely! Can we move the illustration up about 20-32px? It sits a bit low. Otherwise this is awesome work 😄 Thank you! |
Thanks for your feedback @dubielzyk-expensify. I updated |
I like that idea. I also wonder if instead of changing the padding, we just add a line break so it wraps exactly how we want it to? |
True... we could definitely try that out and see what happens! |
@shawnborton @dannymcclain. Thanks for your suggestions. I updated the PR |
Looks good! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the patience @tienifr lets be on a look out for any regressions
given Shawn's approval I will move this ahead to avoid any merge conflicts |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/mountiny in version: 9.0.21-0 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.0.21-4 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.0.21-4 🚀
|
Hmm I am getting some jumpiness when the "not found" message appears under the search input - I thought we fixed that? CleanShot.2024-08-28.at.12.30.09.mp4 |
@tienifr Are you able to look into this one. Can you reproduce? It should have been fixed I believe |
{flattenedSections.allOptions.length === 0 ? ( | ||
renderListEmptyContent() | ||
) : ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line introduces #50577
More details in here: #50577 (comment)
Details
This PR adds illustrations to empty list for create flows:
Fixed Issues
$ #42227
PROPOSAL: #42227 (comment)
Tests
Offline tests
NA
QA Steps
Note
Refer to iOS: mWeb Safari and MacOS: Chrome / Safari screenshots for the latest design.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
Screen.Recording.2024-07-24.at.16.17.14-compressed.mov
MacOS: Chrome / Safari
MacOS: Desktop