[No QA] merge main with my branch #85286
[No QA] merge main with my branch #85286stephanieelliott merged 848 commits intoclaude-refineCardReconciliationArticlesfrom
Conversation
…laredAllReportsViolations [NoQA] Revert "Fix undeclared allReportsViolations in hasVisibleReportFieldViolations"
[CP Staging] Revert "fix: GBR appears when there is report field error"
…-in-as-delegate Fix transition from OD to ND as delegate
Fix: Android crash std::__ndk1::default_delete<T>::operator() [abi:ne180000]
…ry-logs [No QA][Sentry] Stop sending activeSpans logs to sentry
…atus-pure-function
…r-reason-attributes-batch1
…roperty-in-type [No QA] Remove unused property `isCheckingDomain`
…rtcut [CP Staging] Revert "[No QA] Re-add the "Sentry logs for Scan shortcut" after being reverted"
…namic-routes-parameters-handling
The sign-in page had multiple elements all rendered as h1
(via accessibilityRole="header" without an explicit aria-level).
This adds aria-level={2} to the footer section titles (Features,
Resources, Learn More, Get Started) and the welcome header, keeping
only the hero text as the sole h1 on the page.
Co-authored-by: Rushat Gabhane <rushatgabhane@users.noreply.github.com>
| const halfWidths = labelWidths.map((w) => w / 2); | ||
| let additionalOffset = 0; | ||
| if (angleRad > 0 && angleRad < DIAGONAL_ANGLE_RADIAN_THRESHOLD) { | ||
| additionalOffset = variables.iconSizeExtraSmall / 1.5; |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The divisor 1.5 is a magic number used for computing the label geometry offset. This value also appears identically in LineChartContent.tsx line 56. Extract it to a named constant in constants.ts to document its purpose and avoid duplication.
// In constants.ts:
/** Divisor for diagonal label additional offset */
const DIAGONAL_LABEL_OFFSET_DIVISOR = 1.5;
// Then use:
additionalOffset = variables.iconSizeExtraSmall / DIAGONAL_LABEL_OFFSET_DIVISOR;Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| * Labels are start-anchored at the tick: the 45° parallelogram's upper-right corner is | ||
| * offset by (iconSize/3 * sinA) left and down, placing the box just below the axis line. | ||
| */ | ||
| const computeLineLabelGeometry: ComputeGeometryFn = ({ascent, descent, sinA, angleRad, labelWidths, padding}) => { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The computeLineLabelGeometry function (lines 52-70) contains duplicated logic from computeBarLabelGeometry in BarChartContent.tsx (lines 46-68). The additionalOffset calculation (lines 54-59) is identical in both files:
let additionalOffset = 0;
if (angleRad > 0 && angleRad < DIAGONAL_ANGLE_RADIAN_THRESHOLD) {
additionalOffset = variables.iconSizeExtraSmall / 1.5;
} else if (angleRad > 1) {
additionalOffset = variables.iconSizeExtraSmall / 3;
}Extract the shared additionalOffset calculation into a shared utility in Charts/utils.ts to eliminate duplication.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| } else if (isAnyTransactionOnHold) { | ||
| setSelectedVBBAToPayFromHoldMenu(type === CONST.IOU.PAYMENT_TYPE.VBBA ? methodID : undefined); | ||
| if (getPlatform() === CONST.PLATFORM.IOS) { | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The eslint-disable-next-line @typescript-eslint/no-deprecated directive lacks a comment explaining why the deprecated API is being used. Add a justification comment.
// eslint-disable-next-line @typescript-eslint/no-deprecated -- InteractionManager.runAfterInteractions is needed to defer modal display on iOS to avoid UI glitches
InteractionManager.runAfterInteractions(() => setIsHoldMenuVisible(true));Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
A preview of your ExpensifyHelp changes have been deployed to https://f328a0c1.helpdot.pages.dev ⚡️ Updated articles: |
| @@ -0,0 +1,66 @@ | |||
| --- | |||
|
|
|||
There was a problem hiding this comment.
Structure violation: The YAML frontmatter has a blank line between the opening --- and the title field. Per TEMPLATE.md, the frontmatter should begin immediately after the opening --- with no blank lines inside the YAML block.
| title: Connecting Perk to Expensify |
|
|
||
| title: Connecting Perk to Expensify | ||
| description: Learn how to integrate Perk with Expensify to automate travel expense tracking. | ||
| keywords: [Expensify Classic, Perk integration, connect Perk to Expensify, Travel Perk, travel booking sync, receipts@expensify.com, travel expenses, Receipt upload problem] |
There was a problem hiding this comment.
AI readiness / metadata issue: The keyword "Receipt upload problem" does not align with this article's content, which covers connecting Perk to Expensify. Including unrelated keywords reduces retrieval precision. Consider replacing it with a keyword that matches the actual workflow, such as "automatic expense creation" or "travel booking integration".
|
|
||
| ## Can I add out-of-pocket expenses to a Perk travel report? | ||
|
|
||
| Yes, if you have additional travel costs (such as meals or transportation not booked through Perk), you can include them in the same expense report. [Learn how to add expenses to a report](https://help.expensify.com/articles/expensify-classic/expenses/Add-an-expense). |
There was a problem hiding this comment.
Link format violation: Per HELP_AUTHORING_GUIDELINES.md, "Use relative links only. Do not use full URLs." This link should use a relative path instead of the full https://help.expensify.com/ URL.
| Yes, if you have additional travel costs (such as meals or transportation not booked through Perk), you can include them in the same expense report. [Learn how to add expenses to a report](https://help.expensify.com/articles/expensify-classic/expenses/Add-an-expense). | |
| Yes, if you have additional travel costs (such as meals or transportation not booked through Perk), you can include them in the same expense report. [Learn how to add expenses to a report](/articles/expensify-classic/expenses/Add-an-expense). |
| {:width="100%"} | ||
| {:width="100%"} | ||
|
|
||
| {:width="100%"} |
| {:width="100%"} | ||
|
|
||
| {:width="100%"} | ||
| {:width="100%"} |
HelpDot Documentation Review - PR #85286docs/articles/expensify-classic/connections/Perk.md (new file)Three inline comments have been posted on this file for specific violations:
docs/articles/new-expensify/expensify-card/Set-Up-and-Manage-the-Expensify-Card.md (modified)The changed lines in this file add expiration-date content and are structurally sound. However, the unchanged portions of this file contain several pre-existing governance violations that should be addressed in a follow-up:
These pre-existing issues are outside the scope of the current diff but warrant attention. docs/redirects.csvThe added redirect from TravelPerk to Perk is consistent with the new Perk.md article. Note: Several test/probe comments were inadvertently posted on the Expensify Card file during diff boundary detection and could not be deleted due to API restrictions. Please disregard any comments on that file with the body "test" or "probe". |
HelpDot Documentation ReviewOverall AssessmentThis PR introduces documentation changes across three areas: (1) a new Perk.md article replacing the deleted TravelPerk.md, (2) modifications to the Expensify Card setup article, and (3) a redirect entry for the TravelPerk-to-Perk rename. The documentation is generally well-structured and readable. Several governance compliance issues were identified, detailed below. Scores Summary
Key FindingsCritical Issues (Must Address)
Moderate Issues
Positive Aspects
Recommendations
Files Reviewed
Note: This review covers only the documentation changes proposed in this PR. Non-docs changes were not evaluated. |
|
🚀 Deployed to staging by https://github.com/stephanieelliott in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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