App
App copied to clipboard
Expensify
[$500] Feature Request : Value of the notes should be seen without having to click on it
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.37-1 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707241840458699
Action Performed:
- Open the room settings
- Look at the Private notes
Expected Result:
Private notes field should be similar to room description field to see the notes
Actual Result:
User can't see the value of the notes without having to click into it.
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~010e061ef634b80a1b
- Upwork Job ID: 1754989244938903552
- Last Price Increase: 2024-02-06
Triggered auto assignment to @puneetlath (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Job added to Upwork: https://www.upwork.com/jobs/~010e061ef634b80a1b
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Feature Request : Value of the notes should be seen without having to click on it
What is the root cause of that problem?
New feature
What changes do you think we should make in order to solve the problem?
similar to the room description we need to use MenuItemWithTopDescription instead of the MenuItemhere
to do so we need to remove the private notes from the menu items.
then add a MenuItemWithTopDescription similar to the description below the Description item as follows
const userNote = props.report.privateNotes[props.session.accountID] || {};
{!isThread && !isMoneyRequestReport && !ReportUtils.isTaskReport(props.report) && (
<OfflineWithFeedback pendingAction={props.report.pendingFields.privateNotes}>
<MenuItemWithTopDescription
shouldShowRightIcon
interactive
title={userNote.note || ""}
shouldRenderAsHTML
shouldCheckActionAllowedOnPress={false}
description={props.translate('privateNotes.title')}
onPress={() => ReportUtils.navigateToPrivateNotes(props.report, props.session)}
/>
</OfflineWithFeedback>
)}
Proposal
Please re-state the problem that we are trying to solve in this issue.
Feature Request : Value of the notes should be seen without having to click on it
What is the root cause of that problem?
New feature
What changes do you think we should make in order to solve the problem?
To fix this issue we need to make similar component for Private Notes
https://github.com/Expensify/App/blob/fc60fa62c9584f1db0a89ad1a0cf9dd7caa361e7/src/pages/ReportDetailsPage.js#L247-L255
As result
const canEditPrivateNotes = !isThread && !isMoneyRequestReport && !ReportUtils.isTaskReport(props.report);
<MenuItemWithTopDescription
shouldShowRightIcon={canEditPrivateNotes}
interactive={canEditPrivateNotes}
title={report.privateNotes?.[session.accountID]?.note ?? ''}
shouldRenderAsHTML
shouldCheckActionAllowedOnPress={false}
description={props.translate('privateNotes.title')}
onPress={() => ReportUtils.navigateToPrivateNotes(props.report, props.session)}
/>
Plus we need delete this element from menuItems
https://github.com/Expensify/App/blob/fc60fa62c9584f1db0a89ad1a0cf9dd7caa361e7/src/pages/ReportDetailsPage.js#L153-L162
The only problem is that we want to display in the text Since we can have some notes We can use my Notes value or add new text when we have some notes or empty string when nothing
Plus do we want to show this component constantly and disable only interaction, or if canEditPrivateNotes is false, completely hide the element ?
What alternative solutions did you explore? (Optional)
NA
Requirements
-
When a private note is not set, show a menu item -
-
When a private note is set, the note's first line should be visible. Any excess length should be elipsed because notes can be long and we don't want to fill the screen.
-
Use this logic to hide the private notes menu item. Ideally we should put this in
ReportUtilshttps://github.com/Expensify/App/blob/fc60fa62c9584f1db0a89ad1a0cf9dd7caa361e7/src/pages/ReportDetailsPage.js#L153
@puneetlath please let me know if you disagree with the requirements or have anything to add.
Waiting for proposals! 👀
Proposal
Please re-state the problem that we are trying to solve in this issue.
New feature!
What is the root cause of that problem?
We use MenuItem instead of MenuitemWithTopDescription.
What changes do you think we should make in order to solve the problem?
In the ReportDetailsPage:
- Let's remove the PrivateNotes from the mapped Menuitems.
- And add a new custom block in the render
- and create some new variables at the component level to be shared
So: At the top lets check if we have privateNotes, and create shared Props for either MenuItem or MenuitemWithTop Description
const privateNotes = props.report.privateNotes[props.session.accountID].note;
const baseNoteProps = {
key: CONST.REPORT_DETAILS_MENU_ITEM.PRIVATE_NOTES,
isAnonymousAction: false,
title: props.translate('privateNotes.title'),
onPress: () => ReportUtils.navigateToPrivateNotes(props.report, props.session),
shouldShowRightIcon: true,
brickRoadIndicator: Report.hasErrorInPrivateNotes(props.report) ?
CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '',
}
And then lets conditionally render the component based on presence of private notes. Sharing as much props as possible.
{(!isThread && !isMoneyRequestReport && !ReportUtils.isTaskReport(props.report))&& privateNotes ? (
<MenuItemWithTopDescription
description={privateNotes}
shouldParseTitle
titleStyle={styles.flex1}
{...baseNoteProps}
/>
) : (
<MenuItem {...baseNoteProps} />
)}
As always make sure it's type safe, omitted for demonstration purposes. Goal is to share the logic and conditionally render with the least amount of duplication. Triple check the props are being passed from previous to the new, and test the new components in different layouts.
What alternative solutions did you explore? (Optional)
🎀👀🎀 c+ reviewed
i like @jeremy-croff's proposal https://github.com/Expensify/App/issues/35958#issuecomment-1931230457
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
Based on the above screenshots can you confirm this UI?
No pencil icon anymore
It seems to make sense to include the icon on the MenuItem variant atleast.
@jeremy-croff Private notes should be the title and the note text should be subtitle.
It seems to make sense to include the icon on the MenuItem variant atleast.
Good point! i agree that it makes sense to add pencil icon when no note has been set.
cc: @Expensify/design could you please help confirm design here
Attached is the new renderings with the Icon: Thanks for pointing out the title/subtitle. I had also caught it after screenshotting :)
I don't disagree with this being a feature request in general.
Good point! i agree that it makes sense to add pencil icon when no note has been set.
For fields like this we don't really use the pencil icon so I think we should stick with the chevron/caret that's there now which is consistent with other push-to-page.
Designers from @Expensify/design could correct me if I'm wrong.
As far as I understand the request was to make Private Notes similar to the Room Description, in the room description we don't flip the item into MenuItem if the value is empty
Yeah I agree with that, if we're going to treat this just like the room description, we should just use our standard push-to-page row (no pencil icon, just a right chevron as Jon mentioned)
And then I think we should move this towards the top to sit below room description as well? This way all of the push rows sit together in a group.
That being said... I do want to push back on why we're even making this change. @puneetlath I feel like private notes is a bit of a power user feature, do we really want to put it front and center like this? I feel like this feature was mostly built for internal use and it feels a bit strange to me to make it so prominent for everyone.
It just feels weird to me that the description field and the private notes field have different UX, even though they are the exact same type of field. And while private notes are a power user feature, if you do have private notes set, I think it's a big improvement not to have to click into the field in order to see those notes. If you don't set them, then it would be no more prominent than they are today, right?
This is what the two fields currently look like when empty:
This is what they look like with data:
And this is what they look like in "edit" mode:
The room description UX just seems much better to me.
If you don't set them, then it would be no more prominent than they are today, right?
We'd be moving them up from the bottom of the list towards the top to sit underneath description, so that we keep the same types of fields (push rows) in the same grouping.
Private notes also gets tricky because there can be multiple notes on the same thing, right? Like I thought multiple people could add notes to a certain user/room, hence why we opted for the option row to push you out to a different page entirely for all of that.
Also, how are you getting this screenshot here:
It looks like the field is disabled or something? That's definitely a bug if that's the case.
Private notes also gets tricky because there can be multiple notes on the same thing, right? Like I thought multiple people could add notes to a certain user/room, hence why we opted for the option row to push you out to a different page entirely for all of that.
Oh interesting, I didn't know that. I thought private notes could only be seen by me. So you and I can both add private notes to the same room. But we only see our own notes. That's what it seems to indicate based on the description for the field:
Keep notes about this chat here. You are the only person who can add, edit or view these notes.
@garrettmknight can you confirm? I could have sworn private notes was designed in a way to support multiple notes.
For that screenshot, maybe it's an offline with feedback pattern where it hadn't sent to the server? This is what I'm seeing now:
Multiple notes per person per room weren't included in the initial design (doc here/blurb below).
Create a single note for each DM and room for each member
Each room and DM will have a single, continuous note for each member. To maintain consistency with other forms and textboxes, we’ll include a ‘Save’ button and allow users to write in markdown.
Proposal
Please re-state the problem that we are trying to solve in this issue.
We want to see the value of the notes without having to click on it
What is the root cause of that problem?
New feature
What changes do you think we should make in order to solve the problem?
Even though my original proposal got accepted: https://github.com/Expensify/App/issues/35958#issuecomment-1931230457 It seems after design is chiming in, we want to refrain from switching Components. Revisions from original proposal:
- Just use the < MenuItemWithTopDescription /> component for the private notes item, no need for prop sharing and hoisting.
- Reorder the menu item to be below the room description.
What alternative solutions did you explore? (Optional)
@JmillsExpensify @trjExpensify do you have strong feelings here? Personally I feel fairly strongly that we should keep Private Notes behind an option row. It feels like a weird feature to give such prominent real estate too. Especially since this was a feature we built mostly for internal purposes...
I think Shawn is right, it's possible there are multiple notes here because of this:
To give the new owner and/or mentor visibility into the notes for a deal/customer, we’ll allow users with @expensify.com (Mentors) and @team.expensify.com (Guides and AMs) logins to view notes written by @team.expensify.com logins in rooms and DMs they are members of.
Looks like it did get implemented that way here.
Ah ok. If it's possible for there to be multiple notes, then that seems fine to me.