-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Implement animated WorkspacesListPage and LoungeAccessPage #23061
Conversation
There is a minor known issue with the WorkspacePlanet.json animation that shows some lines of magenta during the rotation: MagentaOverflow.movI'm not entirely sure what's causing this but I think it may be intrinsic to the animation itself because that color is not on the view beneath the animation. I don't think this is worth blocking this PR over, and can be addressed in a follow-up. |
Updated (and fixed a bug in |
Looks so good! One thing that I think I missed from the last PR - in the page header, can we make the arrow use the 80% opacity ivory, but the actual header text just use 100% opacity ivory? |
No problem @shawnborton, here's what that looks like: I agree that looks a bit better 👍🏼 |
@shawnborton I also fixed this other bug as requested. |
🧪🧪 Use the links below to test this build in android and iOS. Happy testing! 🧪🧪 |
Sweet, thanks! |
@Beamanator bump for re-review please |
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.
Looks great to me, nice work @roryabraham !
🚀 Deployed to staging by https://github.com/Beamanator in version: 1.3.46-0 🚀
|
🚀 Deployed to staging by https://github.com/Beamanator in version: 1.3.47-0 🚀
|
🚀 Deployed to production by https://github.com/marcaaron in version: 1.3.46-2 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.47-6 🚀
|
const {translate} = useLocalize(); | ||
|
||
if (!user.hasLoungeAccess) { | ||
return <FullPageNotFoundView shouldShow />; |
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 has caused a small regression in #25433
FullPageNotFoundView
isn't wrapped with SafeAreaView
, so it was overlapping with the notification bar on Android.
We resolved this by using <NotFoundPage />
instead, which is FullPageNotFoundView wrapped with ScreenWrapper (it has safe area insets)
Details
Implements the new animated layouts in WorkpacesListPage and LoungeAccessPage. Since they share a very similar layout, I created some utility components to DRY up code between them.
Note that this code includes a workaround for an animation that's not quite the right aspect ratio. We'll clean this up in a follow-up once we have an animation source in a better aspect ratio.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/300446
$ #23209
$ (partial) #12261
Tests / QA Steps
true
and this condition tofalse
.Settings
->Profile
and you should see a menu item forLounge Access
. Click on that to open the Lounge access page.true
./settings/profile/lounge-access
in the browser and verify that you now see a full page not found view in the RHP.true
. Verify that you see a feature list for the workspace page that matches the screenshots below.Settings
->Workspaces
and verify that the workspace list appears below the animations as shown in the screenshots below.Offline tests / QA Steps
(narrow screens only)
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
iOS.mov
Android