-
Notifications
You must be signed in to change notification settings - Fork 104
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.
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
Add Interactive Dialogs example #28
Conversation
@jasonblais Please give this a first round of review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick initial review. Looks good to me. Some minor proposed text adjustments
@jasonblais Just to be clear: All the texts are Copy&Past from https://docs.mattermost.com/developer/interactive-dialogs.html#example. Should they also be updated in the docs? |
Minor text updates. Follow up to mattermost/mattermost-plugin-demo#28
@hanzei Yes, I made an update for three of the texts for docs mattermost/docs@8c6f83d |
Co-Authored-By: hanzei <16541325+hanzei@users.noreply.github.com>
@hanzei Apart from text, is there anything else you'd like me to PM review? |
@jasonblais I update the command to show an help.
Basically: Is the dialog working as you thought it would? Might be a good time to check if Interactive Dialogs work as designed. Please note that a message is shown in chat when you cancel the dialog. |
@hanzei Would you be open to sharing a binary for demo plugin that I can use for testing? |
@jasonblais Sure. Will send your a DM. |
This is looking great. Will make testing significantly easier. 1 - I received a JavaScript error when enabling the plugin you shared. May or may not be related to the changes on this PR. 2 - Would prefer there was an output posted which we already discussed in the GitHub issue. For now, we can just post an output of all the provided information except the email address. 3 - Propose including a reference in README, so that developers and QA are aware of this being available. Further details about interactive dialogs can be directed to https://docs.mattermost.com/developer/interactive-dialogs.html. 4 - Finally, a crash occurs with |
Co-Authored-By: hanzei <16541325+hanzei@users.noreply.github.com>
|
This issue has been automatically labelled "stale" because it hasn't had recent activity. /cc @jasonblais @hanzei |
@hanzei That's okay for me if it's valuable to developers |
@jasonblais Moved the images to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Hanzei, just one text update proposal
Co-Authored-By: Jason Blais <13119842+jasonblais@users.noreply.github.com>
Thanks @hanzei 👍 |
cc @scottleedavis If you have feedback about this PR, feel free to share it. |
@levb @crspeller Gentle reminder to review this PR. Because it touches a lot of code, I would merge it rather sooner then later. Feel free to request a review from someone else, if you are to busy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to have an example of this.
Switching to @ali-farooq0 as a reviewer because of the changed responsibilities. |
I see that you've added the example to https://docs.mattermost.com/developer/interactive-dialogs.html#example |
IMO the page is outdated and the list of integrations very selective. I would rather rework this whole page. |
This PR adds two examples for Interactive Dialogs:
/dialog
(https://docs.mattermost.com/developer/interactive-dialogs.html#example)/dialog no-elements
On cancel a message is posted in the channel.
Fixes mattermost/mattermost#10286