Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MM-19902 - Added example of channel header dropdown menu item #77

Merged
merged 5 commits into from
Dec 19, 2019
Merged

MM-19902 - Added example of channel header dropdown menu item #77

merged 5 commits into from
Dec 19, 2019

Conversation

marianunez
Copy link
Member

Summary

Added example of a menu item in the Channel Header Dropdown that opens the root modal.

Ticket Link

MM-19902

Screenshot

Screen Shot 2019-11-28 at 9 19 11 AM

Related Pull Requests

@marianunez marianunez added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 28, 2019
@marianunez marianunez mentioned this pull request Nov 28, 2019
Merged
jespino
jespino approved these changes Nov 28, 2019
View reviewed changes
Copy link
Member

@jespino jespino left a comment

Choose a reason for hiding this comment

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

LGTM

@hanzei hanzei added the Do Not Merge Should not be merged until this label is removed label Nov 29, 2019
@hanzei hanzei added this to the v0.5.0 milestone Nov 29, 2019
lieut-data
lieut-data approved these changes Dec 2, 2019
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Nicely done!

@marianunez marianunez removed the 2: Dev Review Requires review by a core committer label Dec 2, 2019
@hanzei hanzei removed the Do Not Merge Should not be merged until this label is removed label Dec 6, 2019
@prapti
Copy link

prapti commented Dec 18, 2019 •
edited
Loading

@marianunez The channel header dropdown added does not seem to be within it's own divided section (see screenshot below) as displayed on the example you've posted in the description. Can this be fixed within this PR? I'm also fine with opening a separate ticket to address this, but if it's a small change can it be added?

Screen Shot 2019-12-18 at 5 35 38 PM

prapti
prapti approved these changes Dec 19, 2019
View reviewed changes
Copy link

@prapti prapti left a comment

Choose a reason for hiding this comment

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

Can't repro the UI issue mentioned above. Will open a separate ticket if seen again. Otherwise looks good.

@prapti prapti added QA Review Done PR has been approved by QA 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Dec 19, 2019
@marianunez marianunez merged commit be7e5d2 into master Dec 19, 2019
@marianunez marianunez deleted the MM-19902 branch December 19, 2019 19:00
@ghost
Copy link

ghost commented Jan 16, 2020 •
edited by ghost
Loading

Hi, just wanted to let you know that this PR is causing issues when integrating plugin in mattermost running under docker preview image https://hub.docker.com/r/mattermost/mattermost-preview
two concerns in this PR are: 1) version 5.20 is too high 2) something with the new UI component is breaking the UI completely; the first was addressed by downgrading to 5.18, but the second i didn't have time to dig in
by reverting this PR commit from master I was able to run the plugin

@ghost
Copy link

ghost commented Jan 16, 2020

just out of curiosity: how can version be 5.20 if the last release (today) is 5.19?

@marianunez
Copy link
Member Author

marianunez commented Jan 17, 2020 •
edited
Loading

Hi @kfilimon, thanks for reaching out.

  1. The master branch of Mattermost both webApp and server are currently in v5.20 and we include some of the latest features in this plugin for demo and testing purposes. This is why this plugin currently requires a minimum server version of 5.20.
  2. When you upload a plugin that requires a higher version it should provide an error and not be able to enable. It should not break the UI completely. Thank you for reporting this and will investigate why that is happening for you.

You can use a latest release of the demo plugin that should be compatible with v5.19: https://github.com/mattermost/mattermost-plugin-demo/releases/tag/v0.4.0

@ghost
Copy link

ghost commented Jan 17, 2020

Thank you @marianunez
All clear now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jespino jespino jespino approved these changes

@prapti prapti prapti approved these changes

@lieut-data lieut-data lieut-data approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request QA Review Done PR has been approved by QA
Projects
None yet
Milestone
v0.5.0
Development

Successfully merging this pull request may close these issues.
5 participants
@marianunez @prapti @jespino @lieut-data @hanzei