Closed Bug 1203281 Opened 9 years ago Closed 9 years ago

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

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox43 fixed)

RESOLVED FIXED
mozilla43
Iteration:
43.3 - Sep 21
Tracking Status
firefox43 --- fixed

People

(Reporter: dmosedale, Assigned: dcritchley)

References

()

Details

(Keywords: regression)

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 file, 2 obsolete files)

      No description provided.
Iteration: --- → 43.3 - Sep 21
User Story: (updated)
Target Milestone: --- → mozilla43
Rank: 5
Keywords: regression
Assignee: nobody → david.critchley
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
Target Milestone: mozilla43 → ---
Fix crop on popup menu in contact list for Loop
Attachment #8661951 - Flags: review?(dmose)
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+
Fix crop on popup menu in contact list for Loop
Attachment #8661951 - Attachment is obsolete: true
Attachment #8662602 - Flags: review?(dmose)
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+
Attachment #8662602 - Attachment is obsolete: true
Attachment #8662675 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/a0bc22813c0e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.