Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Fixing MM-14886 without moving back the input inside the menu #2616

Merged
merged 2 commits into from
Apr 6, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Apr 4, 2019

Summary

The problem was the menu being invisible but on top of the arrow button, now is just width 0.

Ticket

MM-14920

@jespino jespino requested review from jwilander and hmhealey April 4, 2019 20:32
@jespino jespino added 2: Dev Review Requires review by a core commiter CherryPick/Candidate A candidate for a quality or patch release, but not yet approved labels Apr 4, 2019
@@ -2041,5 +2041,5 @@
position: absolute;
right: 0;
top: 0;
width: 100%;
width: 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using visibility: hidden make more sense? Or does that have other effects that cause problems here?
https://developer.mozilla.org/en-US/docs/Web/CSS/visibility

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't trust too much in this kind of things because some browsers doesn't allow you to interact with not-visible elements, but yes, we can try with other approaches, anyway we need to test it in all the browsers (my solution works in firefox and chromium for sure)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to using width: 0 I just have the same concern that I'm not sure how all browsers will treat that and if it would still be possible to click or see somehow. I'm 0/5

@jasonblais jasonblais added this to the v5.10.0 milestone Apr 5, 2019
@jespino jespino removed the request for review from hmhealey April 6, 2019 10:46
@jespino jespino added CherryPick/Approved Meant for the quality or patch release tracked in the milestone and removed CherryPick/Candidate A candidate for a quality or patch release, but not yet approved labels Apr 6, 2019
@jespino jespino force-pushed the putting-input-out-again branch from 4fd2187 to 3adc738 Compare April 6, 2019 10:51
@cpanato cpanato merged commit e61f79a into mattermost:master Apr 6, 2019
@cpanato cpanato added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Apr 6, 2019
cpanato pushed a commit that referenced this pull request Apr 6, 2019
* Revert "MM-14886 Move file input back to be child of button (#2614)"

This reverts commit 50c7ae2.

* Fixing original bug without the need of moving the input inside again
@cpanato
Copy link
Contributor

cpanato commented Apr 6, 2019

cherry picked e61f79a into release 5.10 branch

@cpanato cpanato added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Apr 6, 2019
@lindy65 lindy65 added Tests/Done Release tests have been written and removed 4: Reviews Complete All reviewers have approved the pull request labels Apr 8, 2019
@jespino jespino deleted the putting-input-out-again branch April 8, 2019 17:09
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Apr 8, 2019
TranMacTien pushed a commit to Designveloper/mattermost-webapp that referenced this pull request Jun 13, 2019
…most#2616)

* Revert "MM-14886 Move file input back to be child of button (mattermost#2614)"

This reverts commit 50c7ae2.

* Fixing original bug without the need of moving the input inside again
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation Tests/Done Release tests have been written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants