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.

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

Apps documentation - Dynamic form example #919

Closed
wants to merge 8 commits into from

Conversation

mickmister
Copy link
Member

Summary

This PR adds an example of using App forms, and covers these topics:

  • How to render an item in the post dot menu
  • How to open a modal when the post menu item is clicked
  • How to create a form with:
    • Text fields
    • Static select fields
    • Dynamic autocomplete select fields
  • How to redefine the form based on user interaction
  • How to access the associated post after interacting with the post menu using expand

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-36974

TODO

  • Make screenshots of each step of the guide

@mickmister mickmister added 1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor labels Aug 19, 2021
@mickmister mickmister added the preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories label Aug 19, 2021
@github-actions
Copy link

Newest code from mickmister has been published to preview environment for Git SHA 95d2e6c

Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

This doc is amazing and I went over it in great detail. I like the verbiage and grammatically it is solid!

I don't think it is necessary, but you could hyperlink the bullets from the Overview section to the actual sections.

- Text fields
- Static select fields
- Dynamic autocomplete select fields
- How to redefine the form based on user interaction
Copy link
Contributor

Choose a reason for hiding this comment

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

I would mention context.selected_field as a sub-bullet to this one. I see that it is described further down the document, but would be worth adding here.

Copy link
Contributor

Choose a reason for hiding this comment

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

would also mention call.values


These examples assume you have a Node.js App set up using [express](https://github.com/expressjs/express), like the [quick start example]({{< ref "quick-start-js" >}}), or the [example](https://github.com/mattermost/mattermost-plugin-apps/blob/master/dev/node_app/src/app.ts) from the App framework [development environment](https://github.com/mattermost/mattermost-plugin-apps/tree/master/dev).

The examples on this page use types from the [mattermost-redux](https://github.com/mattermost/mattermost-redux) library, namely the App-specific types in [types/apps.ts](https://github.com/mattermost/mattermost-redux/blob/master/src/types/apps.ts); Mattermost types `Post`, `UserProfile`, and `Team`; and the Mattermost JavaScript client. See the end of the guide for the import paths for each of these.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would hyperlink end of the guide to that section of this page.

path: '/forms/create-ticket',
expand: {
acting_user: 'summary', // Expand the acting user so we can get the user's username for our post
team: 'summary', // Expand the current team so we can construct a permalink for the selected post using the team's name
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to use summary in our guides for use cases that don't require the full entity? Does having summary and all cause confusion? Should we be recommending to use summary in general?

It's not clear via docs which fields are included are not when choosing one of these.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also we don't actually need the team name to construct the permalink. We can instead use _redirect in place of the team name.

Copy link

@catalintomai catalintomai Aug 23, 2021

Choose a reason for hiding this comment

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

" Should we be recommending to use summary in general?" - don't see why not - starting small (and grow from there) makes sense to me. OTOH it makes a bit difficult to reconcile expand<->summary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the godocs have a better explanation of what summary provides or not. Maybe we should (verify and) bring that to the developer documentation.

app.post('/bindings', (req, res) => {
const call: AppCallRequest = req.body;

// Use the user id and channel id to dynamically return certain UI elements. Though for this example, we won't use these value.
Copy link

@catalintomai catalintomai Aug 23, 2021

Choose a reason for hiding this comment

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

1/5 - might make sense to not even mention them (if we don't use them - can just confuse the developer) or augment the documentation with an example that actually uses them. Also s/value/values.

app.use('/static', express.static(staticDirectory));
```

We now have an item in the post dropdown menu that says "Create ticket", along with the icon located at `static/create-ticket.png` displayed next to the label. When the post menu item is clicked, our App will receive a [Call]({{< ref "call" >}}) at the endpoint `/forms/create-ticket-from-post/submit`. If we return a form in our response, a modal will be shown to the user with our form in the modal.

Choose a reason for hiding this comment

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

1/5 "that says" - > "with the label" ? (since you mention label in the next phrase).

app.use('/static', express.static(staticDirectory));
```

We now have an item in the post dropdown menu that says "Create ticket", along with the icon located at `static/create-ticket.png` displayed next to the label. When the post menu item is clicked, our App will receive a [Call]({{< ref "call" >}}) at the endpoint `/forms/create-ticket-from-post/submit`. If we return a form in our response, a modal will be shown to the user with our form in the modal.

Choose a reason for hiding this comment

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

1/5 - to be comprehensive, it usually make sense to also mention the negative case ("and if we do not return a form" - you will get an error since the path points to "/forms" or smth. else)


Now let's process the form submitted at the path `/forms/create-ticket/submit`

We've specified in the `expand` clause to include the acting `User` that is submitting the form, as well as the `Team` so we can construct a post's permalink using the team's name.

Choose a reason for hiding this comment

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

" so we can construct a post's permalink using the team's name." - Might make sense to specify/state this goal at the beginning (as a sub-goal of the ticket creation main goal)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would even rephrase this to change the order. Something like:

In the ticket we are going to create, we want to add the permalink to this post. A permalink has the shape baseURL/teamname/post/postID (or something like that). To get the team name, we have added the team as a call expansion.


The user is shown a form with text fields labeled "Summary" and "Description". The Description field is prepopulated with the `Post`'s message.

By defining the form's `Call` with the path `/forms/create-ticket`, we've implicitly registered three endpoints for this form to use:

Choose a reason for hiding this comment

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

might help to specify/mention if these are the only/always three endpoints of if there are more (or less?, depending) for other forms.


### Autocomplete select values

> Note: If we need autocomplete functionality for users or channels, we can set the field's `type` to "user" or "channel", and the from will show an autocomplete selector for the respective field type. The Mattermost server will take care of the autocomplete requests for users and channels. WHen the form is submitted, the field's value will be submitted as an [AppSelectOption](https://github.com/mattermost/mattermost-redux/blob/3d1028034d7677adfda58e91b9a5dcaf1bc0ff99/src/types/apps.ts#L138), where the selected option's `value` is the `id` for the `UserProfile` or `Channel`.
Copy link

@catalintomai catalintomai Aug 23, 2021

Choose a reason for hiding this comment

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

s/from/form
s/take care/handle
s/WHen/When


Now we can respond to user action by rearranging the form's fields when the user selects an Issue Type. Note that this handler will be also called if the user clears the Issue Type field, or selects a new value after already choosing one. We can lock in the chosen value by providing a `readonly: true` property on the field after the user has selected it the first time.

In this example, we should share code between this handler and the `/forms/create-ticket-post-menu/submit` endpoint's handler, since they are returning the same form in both cases.

Choose a reason for hiding this comment

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

0/5 remove "should"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would substitute should for can.

Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

Awesome work Michael! Some comments here and there, but great work in general!


### Manifest

When our App is installed, we will return this Manifest to define our App:
Copy link
Contributor

Choose a reason for hiding this comment

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

In the previous section we said we assume we have an App set up, and I guess that means installed and running. I don't mind being verbose, and explain from the top (i.e. manifest), but in that case, I would make a note here to give more importance to the requested location, and the fact that if you change the manifest, you must reinstall the app.

res.json(callResponse);
});

// When static files are fetched from our App, the request paths are prefixed with `/static`, so our files `icon.png` and `create-ticket.png` will be requested as `static/icon.png` and `static/create-ticket.png`. If we have all of our static assets in one directory, we can serve all of them like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

All these clarifications, if needed, I would make them outside of the code blocks. Again, we are assuming that we have a running app, and static files are not the topic of this document, so not sure if we should go this deep on that.

app.use('/static', express.static(staticDirectory));
```

We now have an item in the post dropdown menu that says "Create ticket", along with the icon located at `static/create-ticket.png` displayed next to the label. When the post menu item is clicked, our App will receive a [Call]({{< ref "call" >}}) at the endpoint `/forms/create-ticket-from-post/submit`. If we return a form in our response, a modal will be shown to the user with our form in the modal.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that semantically, you are returning a form response (which by definition has a form inside to be valid). If you send an OK response with a form, the form will be ignored (as expected).

path: '/forms/create-ticket',
expand: {
acting_user: 'summary', // Expand the acting user so we can get the user's username for our post
team: 'summary', // Expand the current team so we can construct a permalink for the selected post using the team's name
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the godocs have a better explanation of what summary provides or not. Maybe we should (verify and) bring that to the developer documentation.

const call: AppCall = {
path: '/forms/create-ticket',
expand: {
acting_user: 'summary', // Expand the acting user so we can get the user's username for our post
Copy link
Contributor

Choose a reason for hiding this comment

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

As I said before, I feel these clarifications should not be made in code, but outside the code.


When the Issue Type value is selected, our App will receive a request at the endpoint `/forms/create-ticket/form`. The request body will be similar to that of a form submission, but it will also include what field was selected in the `call.selected_field` property.

Note that we can return an entirely new form here. We can also create a flow between several forms by responding to a form submission with a new form.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing right here. I would remove it from here and create a new section somewhere else.

app.post('/forms/create-ticket/form', (req, res) => {
const call: AppCallRequest = req.body;

// For this form, we only have one refresh field, so we should only be receiving requests for that field. This block is here to show that this endpoint could be called for another field, if the form had multiple refresh fields. We would need to handle them individually here when that field's value is selected from the form.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment regarding making this a switch from before.

name: 'summary',
modal_label: 'Summary',
type: 'text',
value: values.summary, // We're using `values.summary` here to make sure we keep the user's entered values in tact when we return this new form
Copy link
Contributor

Choose a reason for hiding this comment

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

r/in tact/intact

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, values is the default value of the form field. I would start with that, and then explain that you put that in there to keep the values intact.

});
```

Now we can respond to user action by rearranging the form's fields when the user selects an Issue Type. Note that this handler will be also called if the user clears the Issue Type field, or selects a new value after already choosing one. We can lock in the chosen value by providing a `readonly: true` property on the field after the user has selected it the first time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Making explanations on the comments and forcing the reader to read those, make the Note that this handler will be also called if the user clears the Issue Type field seems redundant. Having all the explanations in the same place may help the reading flow.


Now we can respond to user action by rearranging the form's fields when the user selects an Issue Type. Note that this handler will be also called if the user clears the Issue Type field, or selects a new value after already choosing one. We can lock in the chosen value by providing a `readonly: true` property on the field after the user has selected it the first time.

In this example, we should share code between this handler and the `/forms/create-ticket-post-menu/submit` endpoint's handler, since they are returning the same form in both cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would substitute should for can.

@hanzei
Copy link
Contributor

hanzei commented Aug 25, 2021

@mickmister Is there anything specific I should review here? There are plenty of people taking a look at this PR right now and I'm not an export in apps interactivity.

@hanzei hanzei removed their request for review August 25, 2021 09:31
Copy link
Contributor

@justinegeffen justinegeffen left a comment

Choose a reason for hiding this comment

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

Some editorial and punctuation feedback, with some typos as well. :)

Each of these examples will be using a modal form to help the user create a ticket in an external system such as Jira or Zendesk. The form will get a little more complex in each example. In this guide we'll cover:

- How to render an item in the post dot menu
- How to open a modal when the post menu item is clicked
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- How to open a modal when the post menu item is clicked
- How to open a modal when the post menu item is selected.

- Text fields
- Static select fields
- Dynamic autocomplete select fields
- How to redefine the form based on user interaction
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- How to redefine the form based on user interaction
- How to redefine the form based on user interaction.

- Dynamic autocomplete select fields
- How to redefine the form based on user interaction
- Using `context` and `expand`:
- How to access the associated post after interacting with the post menu
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- How to access the associated post after interacting with the post menu
- How to access the associated post after interacting with the post menu.

- How to redefine the form based on user interaction
- Using `context` and `expand`:
- How to access the associated post after interacting with the post menu
- How to access other entities in the user's context such as the current `Team` or `Channel`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- How to access other entities in the user's context such as the current `Team` or `Channel`
- How to access other entities in the user's context such as the current `Team` or `Channel`.

// ...
}

// ... Process the submission as usual
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// ... Process the submission as usual
// ... Process the submission as usual.

return;
}

// TicketTag represents a tag in the external ticket system
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TicketTag represents a tag in the external ticket system
// TicketTag represents a tag in the external ticket system.

// TicketTag represents a tag in the external ticket system
let tags: TicketTag[];
try {
// Use some other service to process the autocomplete request
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Use some other service to process the autocomplete request
// Use some other service to process the autocomplete request.

name: 'summary',
modal_label: 'Summary',
type: 'text',
value: values.summary, // We're using `values.summary` here to make sure we keep the user's entered values in tact when we return this new form
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
value: values.summary, // We're using `values.summary` here to make sure we keep the user's entered values in tact when we return this new form
value: values.summary, // We're using `values.summary` here to make sure we keep the user's entered values in tact when we return this new form.

},
};

// Respond with modal form
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Respond with modal form
// Respond with modal form.

@github-actions
Copy link

github-actions bot commented Sep 6, 2021

Newest code from justinegeffen has been published to preview environment for Git SHA b151c07

@hanzei hanzei added the Awaiting Submitter Action Blocked on the author label Nov 8, 2021
@github-actions
Copy link

github-actions bot commented Nov 8, 2021

Newest code from hanzei has been published to preview environment for Git SHA b151c07

@mickmister
Copy link
Member Author

@hanzei Thanks for applying the Awaiting Submitter Action label. FYI I'm waiting to revisit this once all the breaking changes around forms are merged, since a lot of this will need to be modified for those changes.

@hanzei hanzei removed the request for review from levb November 12, 2021 13:16
@github-actions
Copy link

Newest code from justinegeffen has been published to preview environment for Git SHA aac871d

@github-actions
Copy link

Newest code from hanzei has been published to preview environment for Git SHA 9825f9c

@github-actions
Copy link

Newest code from justinegeffen has been published to preview environment for Git SHA bbf9ec9

@cwarnermm
Copy link
Member

@mickmister - Friendly reminder on this PR.

@cwarnermm
Copy link
Member

@mickmister - The dev docs site has seen a lot of changes recently. Given the age of this PR, would you be open to reviewing the changes proposed in this PR against content available in production to confirm whether we want to move ahead with this PR?

@neflyte
Copy link
Contributor

neflyte commented Oct 26, 2022

This PR is mostly or completely covered by #1161.

@mickmister
Copy link
Member Author

Thanks @neflyte, are you thinking we should close this PR?

@cwarnermm
Copy link
Member

@mickmister - Are you open to a quick review of what changed and merged via #1161 and identify any gaps present in this PR that should move forward?

@mickmister
Copy link
Member Author

Closing this, as the content is outdated and we have much more descriptive docs now. We may revisit this to include similar content in the current docs.

@mickmister mickmister closed this Feb 1, 2023
@mattermost-build mattermost-build removed the Awaiting Submitter Action Blocked on the author label Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants