Added Tooltips to the report users' titles. #1632

parasharrajat · 2021-03-04T14:33:36Z

Details

A lot has been going on.

Passing the participantList & tooltipText in options from the createOption method in the OptionsListUtils.js.
Changed the Tooltip Code.
- Now Tooltips are attached to the body of the page on Web/Desktop.
- Mobile devices do not have hover capabilities so tooltips are just empty wrappers.
- TooltIp styling has changed which is required for attaching it to the Body instead of inline.
Added a new Component OptionRowTitle which is used to Show tooltips for Each participant name and .... It does optional tooltip rendering. For places where we don't need tooltips, it renders the Option title(Chat title).
It was very hard to render the tooltips on each user name and keep the ellipsis on the chat title, so I had to use display: inline for tooltip containers. and Shift the tooltip horizontally for the names which were overflowing the container.

Tooltips are added to the following places.

- Show the tooltip for the single user Chats while hovering the title.
- Show the tooltip on the Ellipse for Group chats only if the text is truncated.
- Show the Tooltip with Points 1 and 2 on the search Page.
- Show the Tooltip with Points 1 and 2 on the Report Header.
- Show the Tooltip for the comment author.

Fixed Issues

Fixes #1490

Tests

Hover over the various locations and See the tooltip.

Tested On

Web
Mobile Web
Desktop

screen-.2.mp4

parasharrajat · 2021-03-11T19:54:36Z

Let me know if we need the same behaviour for the Report title in the Header of chat as per the sidebar.
How do you want to list the User Emails while hovering over the Ellipsis? @shawnborton any hint here or some mockup.

parasharrajat · 2021-03-11T20:08:17Z

Ready for Review @roryabraham

roryabraham · 2021-03-11T20:38:44Z

Let me know if we need the same behaviour for the Report title in the Header of chat as per the sidebar.

Yeah, that would be great to see added

src/components/Hoverable/HoverablePropTypes.js

src/components/Tooltip/index.js

src/components/Tooltip/TooltipPropTypes.js

src/pages/home/sidebar/optionPropTypes.js

src/styles/getTooltipStyles.js

src/pages/home/sidebar/OptionRowTitle/index.js

roryabraham · 2021-03-11T21:06:07Z

Let me know if we need the same behaviour for the Report title in the Header of chat as per the sidebar.

Would be great to see this added! cc @shawnborton

shawnborton · 2021-03-11T21:32:06Z

Nice, sounds good to me.

shawnborton · 2021-03-11T21:33:56Z

How do you want to list the User Emails while hovering over the Ellipsis? @shawnborton any hint here or some mockup.

@parasharrajat do you still need some guidance here? Can you show me what it currently looks like?

parasharrajat · 2021-03-11T22:23:39Z

@shawnborton the List looks like the below tooltip.

shawnborton · 2021-03-11T22:28:09Z

Ah apologies, I was mostly responding to the comment about "hovering over the ellipsis" - can you show me what you mean by that?

parasharrajat · 2021-03-11T22:33:23Z

@shawnborton Look at this one.

shawnborton · 2021-03-12T09:46:50Z

Ah, thanks for clarifying. I think with the ellipsis, we'd just show all of the logins in the tooltip as you are already doing.

parasharrajat · 2021-03-16T20:44:34Z

Updated. Thanks.

roryabraham · 2021-03-18T18:38:20Z

@parasharrajat any update on this one? I see there's a merge conflict

parasharrajat · 2021-03-18T20:20:09Z

@roryabraham I just resolved it. Thanks.

src/pages/home/sidebar/optionPropTypes.js

pecanoro · 2021-03-19T10:53:11Z

I am going on vacation next week so I am removing my requested review. However, I took a look at the PR but since I am not as good as @tgolen or @roryabraham with React Native, I will defer the final review to them.

Co-authored-by: Rocio Perez <pecanoro@users.noreply.github.com>

roryabraham

Nice, LGTM! Tested it out a bit too and it seems to work well! All yours @tgolen

roryabraham · 2021-03-19T15:54:32Z

src/pages/home/HeaderView.js

+                {props.report && props.report.reportName && (
+                    <View
+                        style={[
+                            styles.flex1,


NAB maybe rather than composing 4 styles into a new one here you could add a new style that composes these four, like flexRowCenterChildren. Up to you though. I personally think it's better not to compose too many styles inline, because it's equivalent to creating a new style, and we want our JSX to have as little style logic as possible in general.

Hmm. I actually prefer is granular. All four of the classes are generic and it makes sense to create a class if we use all of them together very often. But I prefer tailwind-styled code. Let's see what @tgolen has to say here.

parasharrajat · 2021-03-19T16:49:44Z

Ok. I tried some way but that doesn't seem to work with a dynamic number of names on the chat title. So the current PR is still valid.

tgolen · 2021-03-19T20:04:21Z

OK, thanks for the heads up. Let me know when you want me to look at it again!

…

On Fri, Mar 19, 2021 at 10:50 AM Rajat Parashar ***@***.***> wrote: Please don't merge yet. I am trying some logic that could potentially remove some logic. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1632 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJMAB2W3UF5WS2XDJWAZSLTEN6D3ANCNFSM4YTLQ2EQ> .

parasharrajat · 2021-03-19T20:11:32Z

@tgolen . Sorry I updated the old comment. Here is the update #1632 (comment)

tgolen · 2021-03-19T20:12:35Z

Ah, OK. I'll look at this PR next then.

src/components/Hoverable/HoverablePropTypes.js

src/components/Tooltip/TooltipRenderedOnPageBody.js

src/pages/home/sidebar/OptionRowTitle/index.js

parasharrajat · 2021-03-19T22:03:28Z

@tgolen Updated. Please review.

tgolen

Looks great, thank you for those changes!

tgolen · 2021-03-19T22:17:26Z

@pecanoro All you!

parasharrajat · 2021-03-19T22:20:22Z

@tgolen She is going on leave #1632 (comment)

tgolen · 2021-03-19T22:42:19Z

Ah, thanks. I missed that.

OSBotify · 2021-03-19T22:43:12Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

roryabraham · 2021-03-19T22:49:10Z

OSBotify is lying! He did deploy this to staging 😠 https://github.com/Expensify/Expensify.cash/runs/2152639657?check_suite_focus=true

roryabraham · 2021-03-24T16:05:42Z

Hi @parasharrajat, unfortunately this PR introduced a bug on mobile web. Tooltips in the left-hand-nav are appearing on screen and persisting after the report is opened:

So we need a clever way to make sure that the tooltip isn't opened by taps on touch-screen devices. Any ideas?

marcaaron · 2021-04-02T02:24:54Z

I don't quite understand why we are using a component called OptionRowTitle both in the OptionRow and also in the HeaderView (which has no rows of options).

https://github.com/Expensify/Expensify.cash/blob/106c52b2410bfab984b8cc7ba786fd2affcc29aa/src/pages/home/HeaderView.js#L95-L100

This should have been given a more generic name and hopefully we can get it cleaned up? Thanks.

roryabraham · 2021-04-02T17:08:59Z

Good point, thanks for finding that @marcaaron. I'll create a PR

parasharrajat added 2 commits March 4, 2021 19:58

started Tooltips

cf86c6f

Merge remote-tracking branch 'up/master' into parasharrajat/tooltip

815daf4

parasharrajat requested a review from a team as a code owner March 4, 2021 14:33

botify requested review from pecanoro and removed request for a team March 4, 2021 14:33

parasharrajat added 7 commits March 5, 2021 22:39

New: added tooltip to various parts of the app

21fe2e0

added ellipsis detection

8dece9d

new: Tooltip position for Movable elments

f8f6e98

added tooltip for various parts of the app

9c60e2b

new: tooltip comments & linting

f798513

Merge branch 'master' into parasharrajat/tooltip

de5bbbc

fix: Report is option in headerView

b34457c

parasharrajat changed the title ~~[WIP] : Added Tooltips to the report users' titles.~~ Added Tooltips to the report users' titles. Mar 11, 2021

roryabraham self-requested a review March 11, 2021 20:38

roryabraham assigned parasharrajat Mar 11, 2021

roryabraham requested changes Mar 11, 2021

View reviewed changes

parasharrajat added 2 commits March 12, 2021 03:44

chg: refactor

134367b

fix: styling of tooltip

9b85137

fix: Added tooltip prop & ecllipse only on multiple particpants

e204a20

parasharrajat requested a review from roryabraham March 11, 2021 23:10

refactor Code && showtooltip glitch

21b51ee

parasharrajat mentioned this pull request Mar 17, 2021

Expensify chat - web/desktop, when hovering over a user's name anywhere, show their primary login details #1490

Closed

Merge remote-tracking branch 'up/master' into parasharrajat/tooltip

f4ef0cd

pecanoro removed their request for review March 19, 2021 10:47

pecanoro reviewed Mar 19, 2021

View reviewed changes

src/pages/home/sidebar/optionPropTypes.js Show resolved Hide resolved

removed unnecessary line

237968c

Co-authored-by: Rocio Perez <pecanoro@users.noreply.github.com>

roryabraham previously approved these changes Mar 19, 2021

View reviewed changes

tgolen requested changes Mar 19, 2021

View reviewed changes

refactor

996ae0e

parasharrajat dismissed roryabraham’s stale review via 996ae0e March 19, 2021 22:03

tgolen approved these changes Mar 19, 2021

View reviewed changes

tgolen merged commit 3431bf0 into Expensify:master Mar 19, 2021

github-actions bot locked and limited conversation to collaborators Mar 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Tooltips to the report users' titles. #1632

Added Tooltips to the report users' titles. #1632

parasharrajat commented Mar 4, 2021 •

edited

Loading

parasharrajat commented Mar 11, 2021

parasharrajat commented Mar 11, 2021

roryabraham commented Mar 11, 2021

roryabraham commented Mar 11, 2021 •

edited

Loading

shawnborton commented Mar 11, 2021

shawnborton commented Mar 11, 2021

parasharrajat commented Mar 11, 2021

shawnborton commented Mar 11, 2021

parasharrajat commented Mar 11, 2021 •

edited

Loading

shawnborton commented Mar 12, 2021

parasharrajat commented Mar 16, 2021

roryabraham commented Mar 18, 2021

parasharrajat commented Mar 18, 2021

pecanoro commented Mar 19, 2021

roryabraham left a comment

roryabraham Mar 19, 2021

parasharrajat Mar 19, 2021

parasharrajat commented Mar 19, 2021 •

edited

Loading

tgolen commented Mar 19, 2021 via email

parasharrajat commented Mar 19, 2021

tgolen commented Mar 19, 2021

parasharrajat commented Mar 19, 2021

tgolen left a comment

tgolen commented Mar 19, 2021

parasharrajat commented Mar 19, 2021

tgolen commented Mar 19, 2021

OSBotify commented Mar 19, 2021

roryabraham commented Mar 19, 2021

roryabraham commented Mar 24, 2021

marcaaron commented Apr 2, 2021

roryabraham commented Apr 2, 2021

Added Tooltips to the report users' titles. #1632

Added Tooltips to the report users' titles. #1632

Conversation

parasharrajat commented Mar 4, 2021 • edited Loading

Details

Fixed Issues

Tests

Tested On

parasharrajat commented Mar 11, 2021

parasharrajat commented Mar 11, 2021

roryabraham commented Mar 11, 2021

roryabraham commented Mar 11, 2021 • edited Loading

shawnborton commented Mar 11, 2021

shawnborton commented Mar 11, 2021

parasharrajat commented Mar 11, 2021

shawnborton commented Mar 11, 2021

parasharrajat commented Mar 11, 2021 • edited Loading

shawnborton commented Mar 12, 2021

parasharrajat commented Mar 16, 2021

roryabraham commented Mar 18, 2021

parasharrajat commented Mar 18, 2021

pecanoro commented Mar 19, 2021

roryabraham left a comment

Choose a reason for hiding this comment

roryabraham Mar 19, 2021

Choose a reason for hiding this comment

parasharrajat Mar 19, 2021

Choose a reason for hiding this comment

parasharrajat commented Mar 19, 2021 • edited Loading

tgolen commented Mar 19, 2021 via email

parasharrajat commented Mar 19, 2021

tgolen commented Mar 19, 2021

parasharrajat commented Mar 19, 2021

tgolen left a comment

Choose a reason for hiding this comment

tgolen commented Mar 19, 2021

parasharrajat commented Mar 19, 2021

tgolen commented Mar 19, 2021

OSBotify commented Mar 19, 2021

roryabraham commented Mar 19, 2021

roryabraham commented Mar 24, 2021

marcaaron commented Apr 2, 2021

roryabraham commented Apr 2, 2021

parasharrajat commented Mar 4, 2021 •

edited

Loading

roryabraham commented Mar 11, 2021 •

edited

Loading

parasharrajat commented Mar 11, 2021 •

edited

Loading

parasharrajat commented Mar 19, 2021 •

edited

Loading