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

[MM-18369] Fixed issue where no transition on RHS broke searching on Firefox #3644

Merged
merged 1 commit into from
Sep 12, 2019

Conversation

devinbinnie
Copy link
Member

Summary

My fix here #3594 caused an issue with Firefox, since Firefox does not provide a default computed value for transitions like Chrome does. This PR checks to make sure there is a transition and that it's not the default value. Tested with Chrome, Firefox, Safari and Edge.

Ticket Link

https://mattermost.atlassian.net/browse/MM-18369

@devinbinnie devinbinnie added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Sep 11, 2019
@devinbinnie devinbinnie added this to the v5.15.0 milestone Sep 11, 2019
@devinbinnie devinbinnie requested review from lindy65 and a team September 11, 2019 15:23
@ghost ghost requested review from deanwhillier and gabrieljackson and removed request for a team September 11, 2019 15:23
@devinbinnie devinbinnie added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Sep 11, 2019
@lindy65
Copy link
Contributor

lindy65 commented Sep 11, 2019

Thanks @devinbinnie!

I've tested on Firefox and Search is working without showing any 'searching' spinner.

I have, however, found an issue where, while using the "in:~channel" search, the channel I'm typing into the Search box becomes bolded in the left-hand-side channels list.

I've attached a GIF of this happening (sorry, you have to watch right to the end as I was trying to get it to repro and it only repro'd right at the end!). There is no error in the JS console. I'm not sure whether this is caused by / an issue on this PR?

bolded while searching for IN

@devinbinnie
Copy link
Member Author

@lindy65 Hmm, weird. Definitely a separate issue though. Can you file a new bug?

Copy link
Contributor

@lindy65 lindy65 left a comment

Choose a reason for hiding this comment

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

Thanks @devinbinnie, I've filed https://mattermost.atlassian.net/browse/MM-18495 for the bolding of the channel name in the LHS.

Otherwise this PR looks good to me!

Is this something that a unit test can be added for?

@lindy65 lindy65 removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Sep 11, 2019
@gabrieljackson gabrieljackson added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Sep 12, 2019
@devinbinnie devinbinnie merged commit 5fc04ad into mattermost:master Sep 12, 2019
@devinbinnie devinbinnie deleted the MM-18369 branch September 12, 2019 19:03
@mattermod mattermod 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 Sep 12, 2019
@lindalumitchell lindalumitchell added the Tests/Done Release tests have been written label Sep 13, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Sep 13, 2019
skheria pushed a commit to uber-archive/mattermost-webapp that referenced this pull request Oct 3, 2019
skheria pushed a commit to uber-archive/mattermost-webapp that referenced this pull request Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request 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.

7 participants