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

[MM-28965] - Prevent link overlap in channel header #6546

Merged
merged 3 commits into from
Oct 5, 2020
Merged

[MM-28965] - Prevent link overlap in channel header #6546

merged 3 commits into from
Oct 5, 2020

Conversation

nevyangelova
Copy link
Contributor

Summary

Quick fix for links that overlap in channel header

Ticket Link

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

Fix v5.29

@nevyangelova nevyangelova added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Sep 24, 2020
hmhealey
hmhealey suggested changes Sep 24, 2020
View reviewed changes
components/markdown_image.jsx Outdated
>
{alt}
</a>
/>
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this needs to be changed. If someone links to an unsafe image, it won't show anything now whereas it made a link to that image before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, thanks for clarifying

sass/layout/_headers.scss
@@ -269,6 +269,7 @@
.emoticon {
position: relative;
top: -1px;
overflow: hidden;
Copy link
Member

Choose a reason for hiding this comment

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

I didn't know the header had its own styling for emojis. I'm not at all surprised this broke then

@calebroseland calebroseland removed the 2: Dev Review Requires review by a core commiter label Sep 28, 2020
@amyblais
Copy link
Member

@nevyangelova From your investigation do you know if this is a regression in v5.27?

@nevyangelova
Copy link
Contributor Author

@amyblais it isn't these styles were implemented a year ago.

@ThiefMaster
Copy link
Contributor

I'm relatively sure that this problem only started showing up during one of the last two releases. I had the header that's mentioned in the JIRA ticket for a long time and clicking the link there worked fine.

@nevyangelova
Copy link
Contributor Author

@ThiefMaster it's possible that some styles were removed and revealed this bug but this is the proper fix and it wasn't implemented correctly a year ago.

@ThiefMaster
Copy link
Contributor

I just wanted to point out that it'd probably be a good candidate for a bugfix release if there's going to be one since it's a small but annoying bug.

@nevyangelova
Copy link
Contributor Author

@ThiefMaster makes sense, @amyblais should this go in 5.28?

@amyblais
Copy link
Member

I think v5.29 is ok.

@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 1, 2020
@nevyangelova nevyangelova added this to the v5.29.0 milestone Oct 1, 2020
@nevyangelova
Copy link
Contributor Author

/update-branch

jgilliam17
jgilliam17 approved these changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thank you @nevyangelova
Tested, looks good to merge.

  • Verified header description links are clickable and they are not overlapping.
    Note: The line that appears on mouse hover over linked emoji can't be removed currently and will have to be addressed separately if we decide to change/remove how markdown is processed.

@jgilliam17 jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 5, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@nevyangelova nevyangelova merged commit f58ca3a into master Oct 5, 2020
@nevyangelova nevyangelova deleted the MM-28965 branch October 5, 2020 21:29
Tak-Iwamoto pushed a commit to Tak-Iwamoto/mattermost-webapp that referenced this pull request Oct 6, 2020
Merge branch 'master' of github.com:Tak-Iwamoto/mattermost-webapp int…
d6f9471 
…o MM-20462

* 'master' of github.com:Tak-Iwamoto/mattermost-webapp:
  [MM-22161] - Improve accessibility of channel switcher (mattermost#6199)
  [MM-20460] Migrate 'components/permissions_gates/channel_permission_gate' module and associated tests to TypeScript (mattermost#6601)
  [GH-15724] Remove usage of findDOMNode from Create Team screens (mattermost#6621)
  [MM-29302] Billing Summary Card - No Billing History case (mattermost#6639)
  /files should target=_blank (mattermost#6616)
  MM T134 visual verification of tooltips (mattermost#6604)
  MM-24812 Add e2e tests for adding reactions to existing reactions in archived channel posts (mattermost#6579)
  [MM-20490] Migrate 'components/post_view/combined_user_activity_post' module and associated tests to TypeScript (mattermost#6632)
  MM-24810 Add e2e for reactions for archived channel posts (mattermost#6562)
  [MM-28965] - Prevent link overlap in channel header (mattermost#6546)
  MM-28780: Migrate dotMenu string refs to functional ones (mattermost#6592)
  Cypress/E2E: Fix archived channel specs (mattermost#6617)
  Fixed cypress env for PostgreSQL connection (mattermost#6627)
  Migrate post_list_row component to typescript (mattermost#6593)
  Cypress/E2E: Fix managing bot accounts spec (mattermost#6614)
  Cypress/E2E: Stabilize profile popover spec (mattermost#6612)
  Migrated string refs in signup_email (mattermost#6602)
  Renaming diagnostics-to-telemetry (mattermost#5741)
@amyblais amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Oct 16, 2020
@amyblais
Copy link
Member

/cherry-pick release-5.29

@mattermod
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit to mattermost-build/mattermost-webapp that referenced this pull request Oct 19, 2020
@nevyangelova @mattermost-build
[MM-28965] - Prevent link overlap in channel header (mattermost#6546)
9f55e0e 
* [MM-28965] - Prevent link overlap in channel header

* Add back image link

Co-authored-by: Nevyana Angelova <[email protected]>
Co-authored-by: Mattermod <[email protected]>
(cherry picked from commit f58ca3a)
@mattermost-build mattermost-build mentioned this pull request Oct 19, 2020
Merged
@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 Oct 19, 2020
mattermod pushed a commit that referenced this pull request Oct 19, 2020
@mattermost-build
[MM-28965] - Prevent link overlap in channel header (#6546) (#6844)
4feab22 
Automatic Merge
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 20, 2020
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
@nevyangelova @mattermod @jfrerich
[MM-28965] - Prevent link overlap in channel header (#6546)
0f6e66f 
* [MM-28965] - Prevent link overlap in channel header

* Add back image link

Co-authored-by: Nevyana Angelova <[email protected]>
Co-authored-by: Mattermod <[email protected]>
calebroseland pushed a commit that referenced this pull request Oct 27, 2020
@nevyangelova @mattermod @calebroseland
[MM-28965] - Prevent link overlap in channel header (#6546)
360c59a 
* [MM-28965] - Prevent link overlap in channel header

* Add back image link

Co-authored-by: Nevyana Angelova <[email protected]>
Co-authored-by: Mattermod <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hmhealey hmhealey hmhealey approved these changes

@calebroseland calebroseland calebroseland approved these changes

@jgilliam17 jgilliam17 jgilliam17 approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation
Projects
None yet
Milestone
v5.29.0
Development

Successfully merging this pull request may close these issues.
8 participants
@nevyangelova @amyblais @ThiefMaster @mm-cloud-bot @mattermod @hmhealey @calebroseland @jgilliam17