Repository navigation
[No QA] Docs updates for E/A#100319 Request admin access from Access restricted screen - #100745
Conversation
Co-authored-by: Jakub Steć <jakubstec@users.noreply.github.com>
MelvinBot
added
the
HelpDot
|
@MelvinBot mark this PR as ready for review |
|
🤖 Marked this PR as ready for review. One thing left for you: the view run · no recording available |
Concierge reviewer checklist:
For more detailed instructions on completing this checklist, see How do I review a HelpDot PR as a Concierge Team member? |
|
@shawnborton @ZhenjaHorbach @greg-schroeder One of you needs to 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| 2. Select the domain you are not an admin of. | ||
| 3. On the **Access restricted** screen, under the **Verification required** heading, click **Request admin access**. | ||
|
|
||
| The button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears below the buttons and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. |
There was a problem hiding this comment.
Deterministic writing / vague UI reference: "The button" and "the buttons" are vague references. Per HELPSITE_NAMING_CONVENTIONS.md (Deterministic Writing Rule: be literal, be exact) name the exact control: "The Request admin access button shows a spinner while the request sends..." and specify which buttons the error appears below (e.g., "below Request admin access and Verify yourself").
|
HelpDot Documentation Review Overall Assessment Scores Summary
Key Findings
Recommendations
Files Reviewed
Note: Detailed line-by-line feedback has been provided as inline comments. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23d1d83f92
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| 2. Select the domain you are not an admin of. | ||
| 3. On the **Access restricted** screen, under the **Verification required** heading, click **Request admin access**. | ||
|
|
||
| The button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears below the buttons and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. |
There was a problem hiding this comment.
When an adminship request fails, this tells the member to look below the buttons, but DomainAccessRestrictedPage passes the error as footerComponent, which ConfirmationPage renders above the FixedFooter containing both buttons (src/components/ConfirmationPage.tsx:157-187). The failure-state guidance therefore directs users to the wrong location; change “below” to “above” or omit the location.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — the error renders above the buttons, so the docs text is wrong.
DomainAccessRestrictedPage passes the error as footerComponent (DomainAccessRestrictedPage.tsx:68-75), and ConfirmationPage renders footerComponent pinned to the bottom of the scroll container — a sibling that comes before the FixedFooter holding both buttons (ConfirmationPage.tsx:157-188). The message lands between the description and the buttons.
Suggested replacement for Claim-and-Verify-a-Domain.md:77:
The Request admin access button shows a spinner while the request sends, then changes to a disabled Request sent button. If the request fails, an error message appears above Request admin access and Verify yourself, and you can click Request admin access again to retry. Request admin access is disabled while you are offline.
That wording also clears the sibling review comment about the vague "the button" / "the buttons" references.
I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me.
There was a problem hiding this comment.
I'm still blocked from pushing here — ZhenjaHorbach is a requested reviewer, not the PR author, an assignee, or an existing co-author, so I can't commit the change myself.
Two ways to unblock:
jakubstec(author/assignee) replies here tagging me, or- add
ZhenjaHorbachto the PR assignees and re-run the request.
In the meantime, here's the change ready to commit — it fixes the wrong error location and also clears the sibling "vague the button / the buttons" comment:
| The button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears below the buttons and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. | |
| The **Request admin access** button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears above **Request admin access** and **Verify yourself**, and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. |
I can only make code changes when directed by the PR author, an assignee, or an existing co-author. To have me implement this, the PR assignee or author should reply with a request tagging me.
There was a problem hiding this comment.
@MelvinBot
I was a C+ on the original PR
So I have context
There was a problem hiding this comment.
No doubt you have the context — the gate isn't about that. It only accepts the PR author, an assignee, or an existing co-author, and reviewer status doesn't satisfy it, so I still can't push.
Fastest unblock: jakubstec adds you to the PR assignees, then re-tag me here and I'll commit it. Or jakubstec tags me directly with the request.
If you'd rather not wait, this reply is anchored to the line — you can commit the change straight from here:
| The button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears below the buttons and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. | |
| The **Request admin access** button shows a spinner while the request sends, then changes to a disabled **Request sent** button. If the request fails, an error message appears above **Request admin access** and **Verify yourself**, and you can click **Request admin access** again to retry. **Request admin access** is disabled while you are offline. |
There was a problem hiding this comment.
Done — pushed as 81f1c74. jakubstec is an assignee, so the gate is cleared.
The line now reads:
The Request admin access button shows a spinner while the request sends, then changes to a disabled Request sent button. If the request fails, an error message appears above Request admin access and Verify yourself, and you can click Request admin access again to retry. Request admin access is disabled while you are offline.
That fixes the wrong error location (below → above) and also clears the sibling "vague the button / the buttons" comment by naming both controls.
Why "above" is correct
DomainAccessRestrictedPage passes the error as footerComponent (DomainAccessRestrictedPage.tsx:68-75).
In ConfirmationPage, footerComponent renders inside the view wrapping the ScrollView, absolutely pinned to that view's bottom — a sibling that comes before the FixedFooter holding both buttons (ConfirmationPage.tsx:157-188). So the message lands between the description and the buttons.
AI tests: npm run spell-changed passed (1 file checked, 0 issues). No app code changed, so nothing else was in scope.
Not verified in a live app: reaching the Access restricted screen needs a domain claimed by another account, which the test account can't set up. The above/below conclusion comes from the layout code, not a screenshot.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
…tricted Co-authored-by: Jakub Steć <jakubstec@users.noreply.github.com>
|
🚀 Deployed to staging by https://github.com/stephanieelliott in version: 9.4.92-0 🚀
|
|
@jakubstec Can I check it off the list? |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.92-8 🚀
Bundle Size Analysis (Sentry): |
stephanieelliott
changed the title
Explanation of Change
Help site follow-up to #100319, which reworked the domain Access restricted screen.
Before that PR, the screen showed a static feature list and a single Verify button. It now shows a Verification required heading with two actions: a secondary Request admin access button that calls
requestDomainAdminship(spinner while sending, disabled Request sent once a request is pending, disabled while offline, inline error on failure) and a primary Verify yourself button.docs/articles/new-expensify/domains/Claim-and-Verify-a-Domain.mdonly documented requesting admin access from the Domain exists page in the add-domain flow. It said nothing about the Access restricted screen, which is the path a member hits when they open a domain on the Domains tab that they aren't a Domain Admin for. This PR adds that second path so the article matches the shipped UI.Changes:
## How to request admin access from the Access restricted screenwith the numbered flow, the loading/Request sent/offline/error states, and the Verify yourself alternative.## How to request admin access to a domain someone else set upto## How to request admin access when you add a domain someone else set upso the two entry points are distinguishable, and updated the screenshot placeholder that referenced the old heading.## What happens after you ask for admin accessso the Request sent behavior covers both the Domain exists page and the Access restricted screen.## Why do I see Access restricted when I open a domain?and## Why can't I click Request admin access?.internalScopeandkeywordsfor the new entry point and its labels.Authoring compliance: single workflow, one
#title plus# FAQ, only##sections, every heading task-based or a question, sentence-case bolded UI labels, non-rendered<!-- SCREENSHOT: -->placeholder. Followeddocs/HELPSITE_NAMING_CONVENTIONS.md,docs/HELP_AUTHORING_GUIDELINES.md, anddocs/TEMPLATE.md.UI label verification: Workspaces, the Domains tab, and New domain were confirmed against the live web app. The Access restricted screen itself could not be reached with the test account — reaching it needs a domain claimed by another account, and the add-domain flow gates on account validation. The labels Access restricted, Verification required, Request admin access, Verify yourself, and Request sent were taken verbatim from the merged
src/languages/en.tsstrings in #100319.Fixed Issues
$ #99382
PROPOSAL:
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 changed worked.
Offline tests
N/A — documentation only, no app code changes.
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.AI Tests
Ran locally on the changed file:
npm run spell-changed— passed (1 file checked, 0 issues).Not applicable: no app code, routing config, scripts, patches, or translations changed, so lint, typecheck, Jest, Storybook, React Compiler, and
createDocsRoutes.shwere not in scope.npx prettier --checkcould not run in this environment — the shared.prettierrc.jsfails to resolve@trivago/prettier-plugin-sort-imports.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
@jakubstec, please review the files changed and confirm they reflect the current behavior. Then post a new comment saying
@MelvinBot mark this PR as ready for review.