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

Additional actions in the Comment Thread #163281

Closed
marrej opened this issue Oct 11, 2022 · 7 comments · Fixed by #162750
Closed

Additional actions in the Comment Thread #163281

marrej opened this issue Oct 11, 2022 · 7 comments · Fixed by #162750
Assignees
Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@marrej
Copy link
Contributor

marrej commented Oct 11, 2022

based on discussion in #156885 this issue proposes extension defined additional actions for Comment Threads. (See image below for reference).
Comment

Motivation

To improve the efficiency of work in Code Reviews, extensions providing CommentThreads would like to add direct resolution of Comments without the need to open the Reply button area (Done in the image).
Additionally, there may be extra actions that may be attached to comment, such as proposed edit that can be applied, which conceptually belong together with the Comment, but at the moment do not have a discoverable area to be placed in. (At the moment actions connected to the whole CommentThread could be either placed in the title to be directly visible, relying on icon to be discovered, or in the reply area which is concealed by the additional click)

Technical proposal

To allow extensions to define additionalActions which are always visible in the CommentThread, a new area should be defined, right under the commentReply (Reply Button Area at the bottom of the CommentThread).
This area would contain separator & extension defined actions.
Extensions can define both single commands and submenus.

Submenus will be rendered as Dropdown buttons, where the first action in the submenu will be treated as the primary action - due to the submenus not carrying Command.
If submenu contains only 1 action, then a single button will be created instead of dropdown button.
In case of no action attached to submenu, the submenu action would be displayed.

NOTE: if there are doubts about AdditionalActions being the correct name for this new area, we should change it.

@alexr00
Copy link
Member

alexr00 commented Oct 11, 2022

@daviddossett what do you think of this proposal? The idea is to have a place in a comment thread for actions that aren't well represented by icons.

@daviddossett daviddossett self-assigned this Oct 12, 2022
@daviddossett
Copy link
Contributor

daviddossett commented Oct 12, 2022

If I'm understanding correctly, this is more about just enabling extensions to contribute generic buttons below the input. It sounds like you can just add as many as you want, with some having secondary actions?

Some other thoughts off the top of my head:

  • We should consider if the "Add comment" button should be the only primary button in this view. I'm concerned have more than one in close proximity. See GH example below.
  • @alexr00 would this mean we could move the "Resolve conversation" button to the additional actions row?

CleanShot 2022-10-11 at 20 38 10@2x

CleanShot 2022-10-11 at 20 36 31@2x

@alexr00
Copy link
Member

alexr00 commented Oct 17, 2022

@daviddossett yes, we'd be able to move the "Resolve" button to be always visible. One other thing that came out of discussion on this: If we have these buttons, should be move the "Reply" button/input to be one of these buttons instead of it's own custom button? It currently looks like an input, but if we have this row of buttons then it might make sense to remove the reply input/button and instead have it be an "additional actions" button.

@daviddossett
Copy link
Contributor

My gut reaction is that keeping both the input and the resolve button visible before interacting would be useful:

CleanShot 2022-10-17 at 11 58 26

Showing the input (today it's technically a button but let's say we made it a real textarea component) makes it feel more inviting to click on and start typing vs a button IMO. It also has the benefit of tracking the experience on github.com more closely.

@albertelo
Copy link

If I'm understanding correctly, this is more about just enabling extensions to contribute generic buttons below the input. It sounds like you can just add as many as you want, with some having secondary actions?

To clarify from our end: we would only have one primary action, if we add more actions they would all be secondary. This means one primary action in the whole component, so no multiple primary actions per row

@alexr00 alexr00 added this to the November 2022 milestone Nov 17, 2022
@alexr00 alexr00 added feature-request Request for new features or functionality comments Comments Provider/Widget/Panel issues labels Nov 17, 2022
@alexr00
Copy link
Member

alexr00 commented Nov 17, 2022

We reviewed the UX for the split button yesterday, and after discussing we've decided to not allow the split button right now. We try to keep the split button as a way to show one action with multiple variations. For that reason, we don't want the split button to be an overflow button. Some examples:
image

image

The mock up in #156885 (comment) shows a split button with several disjoint actions. We can't come up with a good use case for the split button with our primary first party consumer of the comments widget, GitHub Pull Requests and Issues, and I'm not aware of another extension that would really benefit from the split button.

We don't want to encourage extension authors to break our paradigm of how a split button should work, which is why, for now at least, we're not going to allow the split button in the comments additional actions.

I'd be happy to hear arguments for why we should support the split button! I expect that this will be an ongoing discussion.

alexr00 added a commit that referenced this issue Nov 17, 2022
and add a max number of 4 actions
Part of #163281
alexr00 added a commit that referenced this issue Nov 17, 2022
@alexr00
Copy link
Member

alexr00 commented Nov 17, 2022

I've also added a limit of 4 buttons to the additional actions. This is similar to what we do with other menus where we allow up to 4, and then when that is exceeded we move actions 4 and up into an overflow menu. Since we haven't defined what an overflow menu means for a row of buttons, the number of buttons is truncated at 4.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants