[$1000] Chat - Report context menu actions are not aligned consistently with hovered messages

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action performed:

1. Open a chat with a decent amount of messages (probably like 10 is enough)
2. Hover your mouse over a few different chats

Expected Result:

context menu aligned consistently every time

Actual Result:

context menu not aligned consistently over every message

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.41-0 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/43996225/208153547-c0434a43-b5c1-4dd5-a8a4-a0612d1b4e07.mov

https://user-images.githubusercontent.com/43996225/208153609-b2ae72e6-2740-41a9-8016-a3f933d06703.mp4

Expensify/Expensify Issue URL: Issue reported by: @Beamanator Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1671181811431219

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0176ee25b5f8c6e294
• Upwork Job ID: 1606391887973285888
• Last Price Increase: 2022-12-23

[JmillsExpensify]

I'm going to take this one over because Greg is on vacay and I'd like to get this one moving for next week.

[JmillsExpensify]

Melvin I just said I got it. 😄

[JmillsExpensify]

Huh, yeah I can also reproduce this pretty reliably. At first I thought it was due to misalignment for multi-line messages, but then I was able to see the same mis-alignment for single messages higher up.

[JmillsExpensify]

I'm going to go ahead and add the external label to get more theories around why this is occurring, and how we can improve the reproduction steps.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0176ee25b5f8c6e294

[melvin-bot[bot]]

Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

[melvin-bot[bot]]

Triggered auto assignment to @robertjchen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Pujan92]

Proposal

Issue: It happens to grouped report action item and occurs from 2nd message onwards due to applying top: -32px.

https://user-images.githubusercontent.com/14358475/209406481-e00273c0-db93-4b03-9dd1-66d58c2b412b.mov

Solution 1: Apply top: -16px or top: -19px(for exact center) for all the items and remove isReportActionItemGrouped parameter from the function which isn't required. We can also take this to the styles.js and remove the function itself as style not depends on any parameter now.

@@ -441,10 +441,11 @@ function getReportActionItemStyle(isHovered = false, isLoading = false) {
  * @param {Boolean} isReportActionItemGrouped
  * @returns {Object}
  */
-function getMiniReportActionContextMenuWrapperStyle(isReportActionItemGrouped) {
+function getMiniReportActionContextMenuWrapperStyle() {
     return {
-        ...(isReportActionItemGrouped ? positioning.tn8 : positioning.tn4),
+        ...positioning.tn4,
         ...positioning.r4,
         position: 'absolute',
         zIndex: 1,
     };

diff --git a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js b/src/pages/home/report/ContextMenu/MiniRepor
tActionContextMenu/index.js
index d90b47d71..b86b9914e 100644
--- a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
+++ b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
@@ -23,7 +23,7 @@ const defaultProps = {
 };

 const MiniReportActionContextMenu = props => (
-    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
+    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle()}>
         {/* eslint-disable-next-line react/jsx-props-no-spreading */}
         <BaseReportActionContextMenu isMini {...props} />
     </View>

---

Solution 2(IMO Preferable): Apply translate style on vertical to make it center consistently irrespective of considering the fixed height for the mini context menu.

@@ -441,10 +441,11 @@ function getReportActionItemStyle(isHovered = false, isLoading = false) {
  * @param {Boolean} isReportActionItemGrouped
  * @returns {Object}
  */
-function getMiniReportActionContextMenuWrapperStyle(isReportActionItemGrouped) {
+function getMiniReportActionContextMenuWrapperStyle() {
     return {
-        ...(isReportActionItemGrouped ? positioning.tn8 : positioning.tn4),
         ...positioning.r4,
+        top: 0,
+        transform: 'translateY(-50%)',
         position: 'absolute',
         zIndex: 1,
     };

https://user-images.githubusercontent.com/14358475/209406492-8860c35e-8123-40ec-b8a3-666a34a73892.mov

[Prince-Mendiratta]

@Pujan92 Hi, really curious how you are able to reply so quickly with a proposal, do you mind sharing some insights? Do you already look at the newly created issues, prepare a proposal for them and the moment they are assigned to external, post the proposal?

[Pujan92]

We can also take this to the styles.js and remove the function itself as style not depends on any parameter now.

diff --git a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
index d90b47d71..5f294c4ae 100644
--- a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
+++ b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
@@ -1,29 +1,23 @@
 import _ from 'underscore';
 import React from 'react';
 import {View} from 'react-native';
-import PropTypes from 'prop-types';
 import {
     propTypes as genericReportActionContextMenuPropTypes,
     defaultProps as GenericReportActionContextMenuDefaultProps,
 } from '../genericReportActionContextMenuPropTypes';
-import * as StyleUtils from '../../../../../styles/StyleUtils';
 import BaseReportActionContextMenu from '../BaseReportActionContextMenu';
+import styles from '../../../../../styles/styles';

 const propTypes = {
     ..._.omit(genericReportActionContextMenuPropTypes, ['isMini']),
-
-    /** Should the reportAction this menu is attached to have the appearance of being
-     * grouped with the previous reportAction? */
-    displayAsGroup: PropTypes.bool,
 };

 const defaultProps = {
     ..._.omit(GenericReportActionContextMenuDefaultProps, ['isMini']),
-    displayAsGroup: false,
 };

 const MiniReportActionContextMenu = props => (
-    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
+    <View style={styles.reportActionMiniContextMenuWrapper}>
         {/* eslint-disable-next-line react/jsx-props-no-spreading */}
         <BaseReportActionContextMenu isMini {...props} />
     </View>
diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js
index 7158f15f3..6bb40add3 100644
--- a/src/pages/home/report/ReportActionItem.js
+++ b/src/pages/home/report/ReportActionItem.js
@@ -232,7 +232,6 @@ class ReportActionItem extends Component {
                                 reportID={this.props.report.reportID}
                                 reportAction={this.props.action}
                                 isArchivedRoom={ReportUtils.isArchivedRoom(this.props.report)}
-                                displayAsGroup={this.props.displayAsGroup}
                                 isVisible={
                                     hovered
                                     && !this.state.isContextMenuActive     
diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js
index 3606cdd4e..d229716dd 100644
--- a/src/styles/StyleUtils.js
+++ b/src/styles/StyleUtils.js
@@ -437,21 +437,6 @@ function getReportActionItemStyle(isHovered = false, isLoading = false) {
     };
 }

-/**
- * Generate the wrapper styles for the mini ReportActionContextMenu.
- *
- * @param {Boolean} isReportActionItemGrouped
- * @returns {Object}
- */
-function getMiniReportActionContextMenuWrapperStyle(isReportActionItemGrouped) {
-    return {
-        ...(isReportActionItemGrouped ? positioning.tn8 : positioning.tn4),
-        ...positioning.r4,
-        position: 'absolute',
-        zIndex: 1,
-    };
-}
-
 /**
  * @param {Boolean} isSmallScreenWidth
  * @returns {Object}
@@ -667,7 +652,6 @@ export {
     getEmojiPickerStyle,
     getLoginPagePromoStyle,
     getReportActionItemStyle,
-    getMiniReportActionContextMenuWrapperStyle,
     getKeyboardShortcutsModalWidth,
     getPaymentMethodMenuWidth,
     getThemeBackgroundColor,
diff --git a/src/styles/styles.js b/src/styles/styles.js
index 9fe3b6234..fb0478231 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -1932,6 +1932,14 @@ const styles = {
         borderColor: themeColors.transparent,
     },

+    reportActionMiniContextMenuWrapper: {
+        ...positioning.tn4,
+        ...positioning.r4,
+        position: 'absolute',
+        zIndex: 1,
+    },
+
     reportActionContextMenuMiniButton: {
         ...spacing.p1,
         ...spacing.mv1,

[syedsaroshfarrukhdot]

Proposal :-

I believe we should use top : -32 to for all MiniReportActionContextMenu on top instead of Top : -16 as using Top : -16 on single item doesn't look good and gives inconsistent feel on single item due to line height the bottom space is very less and doesn't make it centered.

And using top : -16 or transform: 'translateY(-50%) also covers arrow icon in fulfilled request which leads to regression.

diff --git a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
index d90b47d71..32a3ae405 100644
--- a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
+++ b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
@@ -14,16 +14,14 @@ const propTypes = {

     /** Should the reportAction this menu is attached to have the appearance of being
      * grouped with the previous reportAction? */
-    displayAsGroup: PropTypes.bool,
 };

 const defaultProps = {
     ..._.omit(GenericReportActionContextMenuDefaultProps, ['isMini']),
-    displayAsGroup: false,
 };

 const MiniReportActionContextMenu = props => (
-    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
+    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle()}>
         {/* eslint-disable-next-line react/jsx-props-no-spreading */}
         <BaseReportActionContextMenu isMini {...props} />
     </View>
diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js
index 7158f15f3..6bb40add3 100644
:...skipping...
diff --git a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
index d90b47d71..32a3ae405 100644
--- a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
+++ b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
@@ -14,16 +14,14 @@ const propTypes = {

     /** Should the reportAction this menu is attached to have the appearance of being
      * grouped with the previous reportAction? */
-    displayAsGroup: PropTypes.bool,
 };

 const defaultProps = {
     ..._.omit(GenericReportActionContextMenuDefaultProps, ['isMini']),
-    displayAsGroup: false,
 };

 const MiniReportActionContextMenu = props => (
-    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
+    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle()}>
         {/* eslint-disable-next-line react/jsx-props-no-spreading */}
         <BaseReportActionContextMenu isMini {...props} />
     </View>
diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js
index 7158f15f3..6bb40add3 100644
--- a/src/pages/home/report/ReportActionItem.js
+++ b/src/pages/home/report/ReportActionItem.js
@@ -232,7 +232,6 @@ class ReportActionItem extends Component {
                                 reportID={this.props.report.reportID}
                                 reportAction={this.props.action}
                                 isArchivedRoom={ReportUtils.isArchivedRoom(this.props.report)}
-                                displayAsGroup={this.props.displayAsGroup}
                                 isVisible={
                                     hovered
                                     && !this.state.isContextMenuActive
diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js
index 1a14db2e7..f0410703d 100644
--- a/src/styles/StyleUtils.js
+++ b/src/styles/StyleUtils.js
@@ -444,12 +444,11 @@ function getReportActionItemStyle(isHovered = false, isLoading = false) {
 /**
  * Generate the wrapper styles for the mini ReportActionContextMenu.
  *
- * @param {Boolean} isReportActionItemGrouped
  * @returns {Object}
  */
-function getMiniReportActionContextMenuWrapperStyle(isReportActionItemGrouped) {
+function getMiniReportActionContextMenuWrapperStyle() {
     return {
-        ...(isReportActionItemGrouped ? positioning.tn8 : positioning.tn4),
+        ...positioning.tn8,
         ...positioning.r4,
         position: 'absolute',
         zIndex: 1,

Or We can move styles of MiniReportActionContextMenuWrapperStyle in styles.js

diff --git a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
index d90b47d71..16a752a30 100644
--- a/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
+++ b/src/pages/home/report/ContextMenu/MiniReportActionContextMenu/index.js
@@ -6,24 +6,22 @@ import {
     propTypes as genericReportActionContextMenuPropTypes,
     defaultProps as GenericReportActionContextMenuDefaultProps,
 } from '../genericReportActionContextMenuPropTypes';
-import * as StyleUtils from '../../../../../styles/StyleUtils';
 import BaseReportActionContextMenu from '../BaseReportActionContextMenu';
+import styles from '../../../../../styles/styles';

 const propTypes = {
     ..._.omit(genericReportActionContextMenuPropTypes, ['isMini']),

     /** Should the reportAction this menu is attached to have the appearance of being
      * grouped with the previous reportAction? */
-    displayAsGroup: PropTypes.bool,
 };

 const defaultProps = {
     ..._.omit(GenericReportActionContextMenuDefaultProps, ['isMini']),
-    displayAsGroup: false,
 };

 const MiniReportActionContextMenu = props => (
-    <View style={StyleUtils.getMiniReportActionContextMenuWrapperStyle(props.displayAsGroup)}>
+    <View style={styles.MiniReportActionContextMenuWrapperStyle}>
         {/* eslint-disable-next-line react/jsx-props-no-spreading */}
         <BaseReportActionContextMenu isMini {...props} />
     </View>
diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js
index 7158f15f3..6bb40add3 100644
--- a/src/pages/home/report/ReportActionItem.js
+++ b/src/pages/home/report/ReportActionItem.js
@@ -232,7 +232,6 @@ class ReportActionItem extends Component {
                                 reportID={this.props.report.reportID}
                                 reportAction={this.props.action}
                                 isArchivedRoom={ReportUtils.isArchivedRoom(this.props.report)}
-                                displayAsGroup={this.props.displayAsGroup}
                                 isVisible={
                                     hovered
                                     && !this.state.isContextMenuActive
diff --git a/src/styles/styles.js b/src/styles/styles.js
index c2c1837b1..71cfa6c60 100644
--- a/src/styles/styles.js
+++ b/src/styles/styles.js
@@ -2861,6 +2861,12 @@ const styles = {
         fontSize: variables.fontSizeNormal,
         maxWidth: 240,
     },
+    MiniReportActionContextMenuWrapperStyle : {
+         ...positioning.tn8,
+         ...positioning.r4,
+         position: 'absolute',
+         zIndex: 1,
+    }
 };

Using Top : -32 or greater gives more consistent feel or we can decide after design review which looks better and adjust it value accordingly because in -32 it gets very close to arrow icon in some cases.

After Fix:-

[syedsaroshfarrukhdot]

I believe we should also add design label and get opinion of @shawnborton on it also which looks better.

[eVoloshchak]

Thank you all for doing awesome work!

@Pujan92's proposal (Solution 2): Looks good, and centers the menu precisely

But in this case, menu interferes with the arrow

@Pujan92's second proposal:

Has the same issue with arrows, but the menu is a little bit offset from the center

@syedsaroshfarrukhdot's proposal:

Fixes interference with the arrows. However, the uncentered menu looks a bit odd, so I agree with @syedsaroshfarrukhdot, we need input from the design team

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[JmillsExpensify]

@eVoloshchak Thank you for the review! For clarity, are you looking for design feedback specifically on @syedsaroshfarrukhdot's proposal, and perhaps even more specifically the uncentered menu? I know @shawnborton, our designer, is out at the moment but I believe he's on sporadically this week. I'd like to make sure we have a specific question waiting for him, thanks!

[eVoloshchak]

For clarity, are you looking for design feedback specifically on @syedsaroshfarrukhdot's proposal, and perhaps even more specifically the uncentered menu?

The question is: should we implement an uncentered menu or should it be centered and we do something with the arrows?

[JmillsExpensify]

@shawnborton If you wouldn't mind helping move this forward today by chiming in, that'd be appreciated. Thanks!

[bernhardoj]

I think I understand why the context menu is not aligned to center for a group report. It is to prevent the long report text/message to be covered by the menu.

[syedsaroshfarrukhdot]

I just checked slack for reference and noticed we are following current slack behavior to adjust Context Menu Actions Position for long messages and arrow button handling.

Slack Behavior :-

https://user-images.githubusercontent.com/81307212/209782956-5cbc7d42-e74d-4a8a-9f99-11775e12f81f.mov

Expensify Behavior

https://user-images.githubusercontent.com/81307212/209783164-0f79df69-c394-4b78-8c2c-329b30179a2e.mov

I believe this doesn't qualify as a bug and isn't worth fixing it is working completely fine as it is.

Thoughts @JmillsExpensify @eVoloshchak ?

[Pujan92]

I just checked slack for reference and noticed we are following current slack behavior to adjust Context Menu Actions Position for long messages and arrow button handling.

Yes, seems the position of the menu is intentionally(inspired by Slack) made this way.

[JmillsExpensify]

I think you're right! Also tested and saw the same. I'm going to close as a result, and @Beamanator can re-open when he returns if he thinks we're missing something.

[shawnborton]

Hmm not sure why but my screenshots look different than yours - the hover menu doesn't fully cover the arrow, so I think it looks fine as it is here:

[Beamanator]

Ooh now I see why we do this, thanks everyone for the investigation and reasoning 👍