popup menu on contact sometimes mostly cropped at top or bottom of contact list

RESOLVED FIXED in Firefox 43

Status

Hello (Loop)
Client
P1
normal
Rank:
5
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dmose, Assigned: dcritch)

Tracking

({regression})

unspecified
mozilla43
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

(URL)

User Story

As a user, when I popup a menu to edit, remove, or block a contact, the menu should be fully functional and not cropped off the edge of the viewport so that I can actually take that action.

Acceptance criteria:

* all menu options are visible and clickable
* on blur for entire contact (or scroll?), menu is hidden

Technical checklist:

* review the code to all this stuff (it is in contacts.jsx around the logic handling showDropdown and hideDropdown, but it has regressed for some reasons, probably DOM structure changes).

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Reporter)

Updated

2 years ago
Iteration: --- → 43.3 - Sep 21
User Story: (updated)
Target Milestone: --- → mozilla43
(Reporter)

Updated

2 years ago
Rank: 5
Keywords: regression
(Reporter)

Updated

2 years ago
Assignee: nobody → david.critchley
(Reporter)

Comment 1

2 years ago
Moving the boundingClientRect calculations to happen on the contactListWrapper may be the trick here, since it's the thing that is overflowing.

If that fails, it may be worth falling back to whatever the room list does, which doesn't crop the menu at all.
Blocks: 1179193
Having looked at some of this today, I would highly suggest seeing if we could redo this with DropdownMenuMixin. That's what's used for the rooms list and in other places, and it already has some more complicated code for calculations.

It would also likely prevent the errors I'm seeing in the showcase and tests which I'm having to work around for bug 1203098.

Dave - could you base your patches off the one in there that I'll post in a bit - I'm hoping its not too far off landing.
Depends on: 1203098

Updated

2 years ago
Target Milestone: mozilla43 → ---
(Assignee)

Comment 3

2 years ago
Created attachment 8661951 [details] [diff] [review]
Attachment to Bug 1203281 - popup menu on contact sometimes mostly cropped at top or bottom of contact list

Fix crop on popup menu in contact list for Loop
Attachment #8661951 - Flags: review?(dmose)
(Reporter)

Comment 4

2 years ago
Comment on attachment 8661951 [details] [diff] [review]
Attachment to Bug 1203281 - popup menu on contact sometimes mostly cropped at top or bottom of contact list

Review of attachment 8661951 [details] [diff] [review]:
-----------------------------------------------------------------

Please run all the tests using the run-all-loop-tests scripts and fix any issues.

In general, looks great.  Some comments inline.

::: browser/components/loop/content/css/contacts.css
@@ +333,5 @@
>    width: 4px;
>    height: 20px;
>    background-image: url("../shared/img/ellipsis-v.svg");
>    background-size: contain;
>  }

Please leave the blank line for readability.

::: browser/components/loop/content/js/contacts.jsx
@@ +158,5 @@
>        blocked: React.PropTypes.bool,
>        canEdit: React.PropTypes.bool,
> +      eventPosY: React.PropTypes.number.isRequired,
> +      handleAction: React.PropTypes.func.isRequired,
> +      getContainerCoordinates: React.PropTypes.func.isRequired

Please add comments for the props you added.

@@ +187,5 @@
> +        // Position below click area.
> +        menuNode.style.top = this.props.eventPosY + offset + "px";
> +      }
> +    },
> +

I would expect that we'll need some automated test changes; though you may be able to draw from the panel ones for inspiration.

@@ +327,3 @@
>        return (
> +        <li className={contactCSSClass}
> +            onMouseLeave={this._handleMouseOut}>

There are dragons on testing this, onMouseLeave testing has some issues until React 0.14: https://github.com/facebook/react/issues/1297

@@ +346,5 @@
>              ? <ContactDropdown blocked={this.props.contact.blocked}
>                                 canEdit={this.canEdit()}
> +                               eventPosY={this.state.eventPosY}
> +                               handleAction={this.handleAction}
> +                               ref="menu"

This can be deleted, I bet.
Attachment #8661951 - Flags: review?(dmose) → feedback+
(Assignee)

Comment 5

2 years ago
Created attachment 8662602 [details] [diff] [review]
Attachment to Bug 1203281 - popup menu on contact sometimes mostly cropped at top or bottom of contact list

Fix crop on popup menu in contact list for Loop
Attachment #8661951 - Attachment is obsolete: true
Attachment #8662602 - Flags: review?(dmose)
(Reporter)

Comment 6

2 years ago
Comment on attachment 8662602 [details] [diff] [review]
Attachment to Bug 1203281 - popup menu on contact sometimes mostly cropped at top or bottom of contact list

Review of attachment 8662602 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, with one nit, which I'll fix.  r=dmose, assuming the tests pass.

::: browser/components/loop/content/js/contacts.jsx
@@ +194,3 @@
>      render: function() {
>        var cx = React.addons.classSet;
> +      var dropdownClasses = React.addons.classSet({

You can just use cx here, since it's been declared above.  I'll tweak this before landing.
Attachment #8662602 - Flags: review?(dmose) → review+
(Reporter)

Comment 8

2 years ago
Created attachment 8662675 [details] [diff] [review]
Fix crop on popup menu in contact list for Loop
Attachment #8662602 - Attachment is obsolete: true
Attachment #8662675 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/a0bc22813c0e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.