[NO QA] Better RHP view v3 - Implement wide RHP functionality#69668
Conversation
|
@parasharrajat 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] |
Removing the product-PR label for this one. 👍 |
|
@adamgrzybowski is this ready for review? |
|
@parasharrajat, yes, I thought you started your review already 🤔 |
|
I mean to ask whether you fixed the conflicts code and its ready for another review. |
|
@parasharrajat I reacted with thumbs up emoji, but just to be sure you will get the notification 😄 Yes, it is ready for the next round of review |
|
I approved this as I didn't find any issues. Let's wait for the pending comments to be resolved. |
|
Bump @mountiny |
|
@adamgrzybowski can you please sync with main |
…-wide-rhp-functionality
|
@mountiny synced |
mountiny
left a comment
There was a problem hiding this comment.
@adamgrzybowski Are you sure this is NoQA? Some of the changes look quite low level to me
src/libs/Navigation/AppNavigator/Navigators/RightModalNavigator.tsx
Outdated
Show resolved
Hide resolved
| width: isSmallScreenWidth ? '100%' : variables.sideBarWidth, | ||
| height: '100%', | ||
| right: 0, | ||
| position: Platform.OS === 'web' ? 'fixed' : 'absolute', |
There was a problem hiding this comment.
I only see it used once so I am hesitant to add another usage. I think you can create the utils file with the appropriate platforms split and then import these styles from there @adamgrzybowski Could you update two cases from here to use that approach? then we could clean up the other cases of Platform.OS ===
@mountiny, There is no new functionality to test. In theory, everything should be the same as before. But we can run some tests. However, I don't know how to describe them except "make sure that everything works the same as on main" |
There are 5 cases with cc: @mountiny |
|
@mountiny, besides the open discussions, all change requests should be implemented. I can still add videos and test steps if you want |
|
I am already discussing this internally to create a ESLInt rule for |
|
@mountiny, what are the next steps on this one? Merging this PR would unblock the last PR |
…-wide-rhp-functionality
|
@adamgrzybowski the jest is still failing here, can you sync with main again, I dont see this same test failing on main |
…-wide-rhp-functionality
|
@mountiny done |
|
✋ 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.2.9-0 🚀
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.2.11-0 🚀
|
|
It appears like this PR caused #70379. Can anyone please take a look, thanks! |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.2.12-4 🚀
|
Explanation of Change
This PR prepares the functionality of the wide RHP from the navigation perspective. It hasn't been used yet in any flow, so you can't test it or see it in the app. It will be used in the next PR.
It's a part of the bigger chain of changes:
Fixed Issues
$ #64025
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
NO QA
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop