-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Suggested Follow-ups][R1] Update the buttons styling & refactor #80782
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
Changes from all commits
9e1d371
04f812c
6e1f376
19e448f
01d0ab3
1d88a72
4a7e1a4
1933e22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| import {DomUtils, parseDocument} from 'htmlparser2'; | ||
| import type {Followup} from '@libs/ReportActionsUtils'; | ||
| import {getReportActionMessage, isActionOfType} from '@libs/ReportActionsUtils'; | ||
| import CONST from '@src/CONST'; | ||
| import type {OnyxInputOrEntry, ReportAction} from '@src/types/onyx'; | ||
| import type {Followup} from './ReportActionsUtils'; | ||
| import {getReportActionMessage, isActionOfType} from './ReportActionsUtils'; | ||
|
|
||
| /** | ||
| * Checks if a report action contains actionable (unresolved) followup suggestions. | ||
|
|
@@ -22,36 +23,28 @@ function containsActionableFollowUps(reportAction: OnyxInputOrEntry<ReportAction | |
| return !!followups && followups.length > 0; | ||
| } | ||
|
|
||
| // Matches a <followup-list> HTML element and its entire contents. (<followup-list><followup><followup-text>Question?</followup-text></followup></followup-list>) | ||
| const followUpListRegex = /<followup-list(\s[^>]*)?>[\s\S]*?<\/followup-list>/i; | ||
| /** | ||
| * Parses followup data from a <followup-list> HTML element. | ||
| * @param html - The HTML string to parse for <followup-list> elements | ||
| * @returns null if no <followup-list> exists, empty array [] if the followup-list has the 'selected' attribute (resolved state), or an array of followup objects if unresolved | ||
| */ | ||
| function parseFollowupsFromHtml(html: string): Followup[] | null { | ||
| const followupListMatch = html.match(followUpListRegex); | ||
| if (!followupListMatch) { | ||
| const doc = parseDocument(html); | ||
| const followupListElements = DomUtils.getElementsByTagName('followup-list', doc, true); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this work on native too?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looked good when I was testing |
||
| if (followupListElements.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // There will be only one follow up list | ||
| const followupListHtml = followupListMatch[0]; | ||
| // Matches a <followup-list> element that has the "selected" attribute (<followup-list selected>...</followup-list>). | ||
| const followUpSelectedListRegex = /<followup-list[^>]*\sselected[\s>]/i; | ||
| const hasSelectedAttribute = followUpSelectedListRegex.test(followupListHtml); | ||
| if (hasSelectedAttribute) { | ||
| const followupList = followupListElements.at(0); | ||
| if (!followupList) { | ||
| return null; | ||
| } | ||
| if (DomUtils.hasAttrib(followupList, 'selected')) { | ||
| return []; | ||
| } | ||
|
|
||
| const followups: Followup[] = []; | ||
| // Matches individual <followup><followup-text>...</followup-text></followup> elements | ||
| const followUpTextRegex = /<followup><followup-text>([^<]*)<\/followup-text><\/followup>/gi; | ||
| let match = followUpTextRegex.exec(followupListHtml); | ||
| while (match !== null) { | ||
| followups.push({text: match[1]}); | ||
| match = followUpTextRegex.exec(followupListHtml); | ||
| } | ||
| return followups; | ||
| const followupTextElements = DomUtils.getElementsByTagName('followup-text', followupList, true); | ||
| return followupTextElements.map((el) => ({text: DomUtils.textContent(el)})); | ||
| } | ||
| export {containsActionableFollowUps, parseFollowupsFromHtml}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| /** | ||
| * Used for generating preview text in LHN and other places where followups should not be displayed. | ||
| * Implemented here instead of ReportActionFollowupUtils due to circular ref | ||
| * @param html message.html from the report COMMENT actions | ||
| * @returns html with the <followup-list> element and its contents stripped out or undefined if html is undefined | ||
| */ | ||
| function stripFollowupListFromHtml(html?: string): string | undefined { | ||
| if (!html) { | ||
| return; | ||
| } | ||
| // Matches a <followup-list> HTML element and its entire contents. (<followup-list><followup><followup-text>Question?</followup-text></followup></followup-list>) | ||
| const followUpListRegex = /<followup-list(\s[^>]*)?>[\s\S]*?<\/followup-list>/i; | ||
| return html.replace(followUpListRegex, '').trim(); | ||
| } | ||
|
|
||
| export default stripFollowupListFromHtml; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import Onyx from 'react-native-onyx'; | ||
| import type {Ancestor} from '@libs/ReportUtils'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Report, ReportAction} from '@src/types/onyx'; | ||
| import type {Timezone} from '@src/types/onyx/PersonalDetails'; | ||
| import {addComment, buildOptimisticResolvedFollowups} from '.'; | ||
|
|
||
| /** | ||
| * Resolves a suggested followup by posting the selected question as a comment | ||
| * and optimistically updating the HTML to mark the followup-list as resolved. | ||
| * @param report - The report where the action exists | ||
| * @param notifyReportID - The report ID to notify for new actions | ||
| * @param reportAction - The report action containing the followup-list | ||
| * @param selectedFollowup - The followup question selected by the user | ||
| * @param timezoneParam - The user's timezone | ||
| * @param ancestors - Array of ancestor reports for proper threading | ||
| */ | ||
| function resolveSuggestedFollowup( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that this is moved to seprate file, can we fully deprecate this in actions/Report.ts?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oooops... done 😄 |
||
| report: OnyxEntry<Report>, | ||
| notifyReportID: string | undefined, | ||
| reportAction: OnyxEntry<ReportAction>, | ||
| selectedFollowup: string, | ||
| timezoneParam: Timezone, | ||
| ancestors: Ancestor[] = [], | ||
| ) { | ||
| const reportID = report?.reportID; | ||
| const reportActionID = reportAction?.reportActionID; | ||
|
|
||
| if (!reportID || !reportActionID) { | ||
| return; | ||
| } | ||
|
|
||
| const resolvedAction = buildOptimisticResolvedFollowups(reportAction); | ||
|
|
||
| if (!resolvedAction) { | ||
| return; | ||
| } | ||
|
|
||
| // Optimistically update the HTML to mark followup-list as resolved | ||
| Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { | ||
| [reportActionID]: resolvedAction, | ||
| }); | ||
|
|
||
| // Post the selected followup question as a comment | ||
| addComment(report, notifyReportID ?? reportID, ancestors, selectedFollowup, timezoneParam); | ||
| } | ||
|
|
||
| export default resolveSuggestedFollowup; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -928,6 +928,24 @@ const staticStyles = (theme: ThemeColors) => | |
| overflow: 'hidden', | ||
| }, | ||
|
|
||
| actionableItemButton: { | ||
| paddingTop: 8, | ||
| paddingBottom: 8, | ||
| backgroundColor: 'transparent', | ||
| borderWidth: 1, | ||
| borderColor: theme.border, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when we're hovering the message change this to button press Same as on the expense
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated look cc @dubielzyk-expensify Screen.Recording.2026-01-28.at.13.59.05.mov |
||
| alignItems: 'flex-start', | ||
| borderRadius: variables.componentBorderRadiusMedium, | ||
| }, | ||
|
|
||
| actionableItemButtonBackgroundHovered: { | ||
| borderColor: theme.buttonPressedBG, | ||
| }, | ||
|
|
||
| actionableItemButtonHovered: { | ||
| borderWidth: 1, | ||
| }, | ||
|
|
||
| hoveredComponentBG: { | ||
| backgroundColor: theme.hoverComponentBG, | ||
| }, | ||
|
|
||
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 PR introduced a bug where suggested response button text gets clipped instead of wrapping to the next line, because
styles.breakWordwas not applied to the button text styles. Fixed in #82113 / #83048