Make Certinia prerequisites step content scrollable in landscape#95515
Conversation
Co-authored-by: Eskalifer1 <Eskalifer1@users.noreply.github.com>
|
🤖 I looked into the failing checks. There's no code fix to push. The only failing check is
Every other check is passing or still pending — no lint, type, test, or build failure. What's needed (human action): the author checklist and the To clear this check:
Once the boxes are checked, the checklist check will pass on its next run. |
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Explanation of ChangeThe Certinia prerequisites setup steps had no scroll container. This wraps the title and step content in a Fixed Issues$#95416 Tests
Precondition:
Offline testsN/A - no changes related to Onyx QA Steps// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / Safari |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp95515-andoid-native.movAndroid: mWeb Chrome95515-android-web.moviOS: HybridApp95515-ios-native.moviOS: mWeb Safari95515-ios-web.movMacOS: Chrome / Safari95515-web.mov |
|
@MelvinBot Mark this PR as ready for review |
|
@Eskalifer1 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] |
|
🤖 Done — this PR is now marked as Ready for review. |
Eskalifer1
left a comment
There was a problem hiding this comment.
LGTM!
Here is author checklist: #95515 (comment)
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.29-0 🚀
|
|
🤖 Help site review: no docs changes required. This PR is a pure responsive-layout fix. It wraps the Certinia prerequisites step title and content in a No user-facing feature, workflow, label, button, tab name, or setting was added or changed — the same three prerequisite steps ( I reviewed the existing Certinia help content (e.g. If you believe a specific article should be updated, reply with |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.31-0 🚀
|
|
🤖 No help site changes required. This PR is a purely visual layout fix. It wraps the Certinia prerequisites step content in a The existing Certinia help articles under No draft PR was created. @Eskalifer1, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.31-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
The Certinia prerequisites setup steps had no scroll container.
CertiniaPrerequisitesPagerendersConnectionLayoutwithshouldUseScrollView={false}(so itsFixedFooter/mtAutoCTA stays pinned to the bottom), andCertiniaPrerequisitesStepitself only used plainViews. In portrait the content fits, but in landscape the shorter viewport clips the step content (most notably theINSTALL_BUNDLEstep, which stacks twoRenderHTMLblocks) and there is no way to reach it.This wraps the title and step content in a
ScrollViewinsideCertiniaPrerequisitesStep, leaving theFixedFooteroutside as a sibling so the CTA stays pinned.contentContainerStyle={styles.flexGrow1}keeps the scroll content at least the height of the viewport, so portrait layout is unchanged (footer still pinned), while landscape now scrolls. Because the fix lives in the shared step component, all three prerequisite steps (INSTALL_BUNDLE,SETUP_CONTACTS,OAUTH) are covered at once.Fixed Issues
$ #95416
PROPOSAL: #95416 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validates your changes worked.
Offline tests
Same as tests.
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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