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

MSC2790: Modal widgets (acquiring user input from a widget) #2790

Open
wants to merge 17 commits into
base: old_master
Choose a base branch
from

Conversation

turt2live
Copy link
Member

@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels Sep 24, 2020
@turt2live turt2live added this to Awaiting SCT input in Spec Core Team Backlog Sep 24, 2020
@turt2live
Copy link
Member Author

Implementation: matrix-org/matrix-react-sdk#5252

@turt2live
Copy link
Member Author

I'm relatively confident in this and the individuals who have had the pleasure of reading this haven't tried to suffocate me with negative feedback, so I think this is ready for FCP.

Under our new guidelines we'd normally request explicit review from a relevant spec core team member on MSCs first, but seeing as the current expert is me and no one else is volunteering, I'm essentially electing the entire core team as the secondary expert for this area.

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Oct 1, 2020

This FCP proposal has been cancelled by #2790 (comment).

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Oct 1, 2020
@turt2live turt2live moved this from Awaiting SCT input to Ready for FCP ticks in Spec Core Team Backlog Oct 2, 2020
@t3chguy
Copy link
Member

t3chguy commented Oct 8, 2020

I sketched this up into a Sequence diagram for simplification.

image

title Modal Widgets

note over Widget,Client: standard widget init

Widget->+Client: open_modal ({...})
note right of Client: Client spawns the Modal Widget
Client-->-Widget: ACK

note over Client,Modal: standard session init

Client->+Modal: widget_config ({...})
Modal-->-Client: ACK

note right of Client: User presses a button
Client->+Modal: button_pressed ({id: ...})
Modal-->-Client: ACK

note left of Modal: Modal chooses to close with data
Modal->+Client: close_modal (D)
Client-->-Modal: ACK
note right of Client: Client destroys the Modal Widget

Client->+Widget: close_modal (D)
Widget-->-Client: ACK

proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
@matrix-org matrix-org deleted a comment from t3chguy Oct 23, 2020
Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

I don't know a whole lot about widgets, but it looks plausible.

proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

I got some way through reviewing this but then got distracted by the infinite loop bug, so posting what I have

proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
A modal widget could instead be the whole dialog (title, buttons - everything), however this has a
number of concerns relating to styling the buttons for every single client possible. The widget API
would also need to be extended to pass through theme information, which it may very well want to do
anyways, but would still be added complexity for a simple form-style widget.
Copy link
Member

Choose a reason for hiding this comment

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

This is true for any other type of widget though? Does it not have all the same problems anyway for displaying the URL part of the modal widget?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is true for any other type of widget though?

Yes, however modal widgets won't always be a modal dialog. On mobile clients for instance, it might be desirable to show a card or pop-in effect rather than a true dialog. In these cases, the widget would have to consider the brand, theme, and platform of the app which leads to infinite amounts of potential CSS. Instead, we pull the buttons and some other basic information out so the client can render it as natively as possible.

Does it not have all the same problems anyway for displaying the URL part of the modal widget?

which problems are you thinking of?

Copy link
Member

Choose a reason for hiding this comment

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

Taking the example of the calendar add on mobile, would you not still have to have infinite amounts of CSS for the event details to match the brand / theme / etc? ie. you'll get a native, "New Event Details", native buttons and then the time / place / description in a white box in Times New Roman.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessarily, if taking the perspective of a cross-platform designer. A widget is typically displayed in a context where the user is aware that it's not native UI, so variances (or even completely different themes) are acceptable. A modal widget would want to provide as much native UI as possible though to give a better user experience within the app. It's much less of a concern for web apps as the CSS becomes super easy to do, but for iOS, Android, and other mobile devices there's not really a good CSS library that makes things appear as other elements.

It's a bit convoluted, but it also plays into a theory that clients can ignore the url and just natively render the widget (form) if they understand enough of the detail.

Choose a reason for hiding this comment

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

If helpful for extra context; the intent here is for modal widgets to default to native interactions for the call to actions on the platform they're being viewed on.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I think the 'type' that allows clients to know what the content should be and render it natively is what I was missing here. The only mention I can see of this is in the real-world example part?

Copy link
Member Author

Choose a reason for hiding this comment

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

@dbkr I've added a bit of clarifying text to the end of this paragraph. Let me know if it makes more (or less) sense.

proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
proposals/2790-modal-widgets.md Outdated Show resolved Hide resolved
Co-authored-by: Hubert Chathi <hubert@uhoreg.ca>
@turt2live
Copy link
Member Author

@uhoreg as the only other person who has ticked for FCP, it's only fair that I mention I've increased the scope of this slightly to support disabling buttons as well.

@turt2live
Copy link
Member Author

Widgets are likely to change before this is worth putting in the spec. Pulling out of FCP.

@mscbot fcp cancel

@mscbot mscbot added proposal-in-review and removed disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Mar 16, 2021
@turt2live turt2live removed this from Ready for FCP ticks in Spec Core Team Backlog Mar 16, 2021
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal widgets anything to do with widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants