-
Notifications
You must be signed in to change notification settings - Fork 29
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
Introduce ReadOnly property for form fields #165
Conversation
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.
Are there client changes required for this?
@@ -22,6 +23,14 @@ const ( | |||
|
|||
// A mattermost channel reference (~name). | |||
FieldTypeChannel FieldType = "channel" | |||
|
|||
TextFieldSubtypeInput TextFieldSubtype = "input" |
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.
According to https://developers.mattermost.com/integrate/apps/interactivity/ the field is named text
. Are the docs wrong?
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.
@hanzei The field type is text
, but the text subtype can be any that are listed here: https://github.com/mattermost/mattermost-webapp/blob/74cbcb7c3f0dff7e21ecb88776b8841dd1fa55e6/components/widgets/settings/text_setting.tsx#L8
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.
It is already documented in the interactivity docs https://github.com/mattermost/mattermost-developer-documentation/blob/master/site/content/integrate/apps/interactivity/_index.md#modal-forms
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.
Yep, but it's documented as text
there not input
. Fixed in 3cda372
(#805)
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.
Tested and passed
- Tested using Michael's new autolink app Convert Autolink plugin into an App, and introduce autolink edit modal mattermost-community/mattermost-plugin-autolink#159
- Confirmed that read only fields are behaving as expected
- Tested on webapp and mobile to ensure client side support for both
- No issues found
LGTM!
@@ -22,6 +23,14 @@ const ( | |||
|
|||
// A mattermost channel reference (~name). | |||
FieldTypeChannel FieldType = "channel" | |||
|
|||
TextFieldSubtypeInput TextFieldSubtype = "input" |
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.
Yep, but it's documented as text
there not input
. Fixed in 3cda372
(#805)
Summary
The
ReadOnly
field is already supported by the webapp and mobile app. This PR adds the missingReadOnly
field on the backend, as well as adding some constants for field types.Ticket Link
Related Pull Requests
mattermost-community/mattermost-plugin-autolink#159