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

Allow for details in message options #125750

Closed
jrieken opened this issue Jun 8, 2021 · 10 comments
Closed

Allow for details in message options #125750

jrieken opened this issue Jun 8, 2021 · 10 comments
Assignees
Labels
api-finalization feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jun 8, 2021

The MessageOptions-type allows to tweak the behavior showInformationMessage and friends. We should also allow for a details property which are then rendered nicer for modal dialogs.

@jrieken jrieken self-assigned this Jun 8, 2021
@jrieken jrieken added this to the June 2021 milestone Jun 8, 2021
@jrieken jrieken added api-finalization feature-request Request for new features or functionality workbench-notifications Notification widget issues labels Jun 8, 2021
jrieken added a commit that referenced this issue Jun 15, 2021

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@jrieken
Copy link
Member Author

jrieken commented Jun 15, 2021

@bpasero I am also looking into supporting detail for non-modal messages. I was planning to concat the detail to the message, separated by a new line. However, it seems that newline are being stripped from notifications. Is that on purpose? Any other ideas?

@bpasero
Copy link
Member

bpasero commented Jun 15, 2021

@jrieken this was mainly to prevent extensions from abusing notifications from showing rich content in notifications. I do not think that notifications are the right widget to show lots of text, specifically because the area to show content is so small.

Why not limit detail to dialogs only? There it makes a lot more sense to me because visually details look different from the message text.

@jrieken
Copy link
Member Author

jrieken commented Jun 15, 2021

Why not limit detail to dialogs only?

My worry is that extensions, that want detail, will now use modal dialogs instead of notifications... But I am also fine to wait for that to happen

@bpasero
Copy link
Member

bpasero commented Jun 15, 2021

Yup, also fine for me. My rationale is that a misbehaving extension that pops up many modal dialogs will get either uninstalled or will start to see lots of issues reported by users as a consequence.

Btw if it makes the API any easier, we could still allow details in notifications but simply append that content to the message so that it is still presented, just visually a bit different compared to dialogs, which I think is fine.

@jrieken
Copy link
Member Author

jrieken commented Jun 15, 2021

but simply append that content to the message so that it is still presented, just visually a bit different compared to dialogs, which I think is fine.

Append how? I tried the newline separation which didn't work. I don't think that we can use dot or something else

@bpasero
Copy link
Member

bpasero commented Jun 15, 2021

Yeah just literally on the extension host even, concatenating the message + detail. It needs some heuristics I guess, e.g. if message does not end with a ., append one and then put details, something like: message + . + detail?

jrieken added a commit that referenced this issue Jun 15, 2021

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@jrieken
Copy link
Member Author

jrieken commented Jun 16, 2021

Based on @mjbvz suggestion: showing "More" which shows details in a modal

Screen.Recording.2021-06-16.at.15.06.43.mov

@bpasero
Copy link
Member

bpasero commented Jun 16, 2021

Interesting idea, should probably discuss in a UX call. I am not sure how obvious it is what "More" means in the context alongside the other buttons (because they are all in the same hierarchy). And then in the modal dialog it maybe less obvious that only the details are showing.

cc @misolori

@jrieken
Copy link
Member Author

jrieken commented Jun 17, 2021

Decision is to not support detail for notification messages for now.

@jrieken jrieken closed this as completed Jun 17, 2021
@jrieken jrieken added the verification-needed Verification of issue is requested label Jun 28, 2021
@jrieken
Copy link
Member Author

jrieken commented Jun 28, 2021

To verify use the yeoman extension sample and add MessageOptions with detail (and modal)

@rzhao271 rzhao271 added the verified Verification succeeded label Jun 29, 2021
@jrieken jrieken added the on-release-notes Issue/pull request mentioned in release notes label Jul 1, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues
Projects
None yet
Development

No branches or pull requests

3 participants