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

Move accept/discard button of refactor preview into panel #141065

Closed
jrieken opened this issue Jan 20, 2022 · 9 comments
Closed

Move accept/discard button of refactor preview into panel #141065

jrieken opened this issue Jan 20, 2022 · 9 comments
Assignees
Labels
feature-request Request for new features or functionality polish Cleanup and polish issue ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded workspace-edit
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jan 20, 2022

The buttons to accept/discard refactorings aren't easy to discover.

@lychung7 proposed to move them into the view (similar to the ports view) and I think we should explore that.

buttons fixed to bottom of panel

@jrieken jrieken self-assigned this Jan 20, 2022
@jrieken jrieken added polish Cleanup and polish issue ux User experience issues workspace-edit labels Jan 20, 2022
@jrieken jrieken added this to the February 2022 milestone Jan 20, 2022
@jrieken
Copy link
Member Author

jrieken commented Jan 24, 2022

Alternative idea: use the existing buttons location but make the rendering much more prominent, e.g so

buttons fixed to bottom of panel

@isidorn
Copy link
Contributor

isidorn commented Jan 24, 2022

I agree that currently the Apply and Discard buttons are not discoverable.
I prefer the first proposed solution - to show the buttons in the view (below the tree). We already have some pattern where we render buttons below the tree - Call Stack view for "Load more stack frames".
We do not have any patterns where we render buttons like that in the title area, so I am against that. Also I think it would be less discoverable.

@jrieken
Copy link
Member Author

jrieken commented Feb 5, 2022

Pushed a first version of this. @misolori I was trying to use the "secondary button" style here but than the "discard" button with have no styles (transparent background). Is there another color or property that I should be using?

Screen Shot 2022-02-05 at 15 43 04

@miguelsolorio
Copy link
Contributor

Not sure, I know when I tested the secondary buttons in the notification center, the secondary: true worked fine:

const buttonOptions = { title: true, /* assign titles to buttons in case they overflow */ };

CleanShot 2022-02-07 at 12 14 22@2x

@isidorn
Copy link
Contributor

isidorn commented Feb 8, 2022

@jrieken can we not use the checkmark before the "Apply"?
Before the Discard there is no icon so it is a bit inconsistent. Also we do not use Icon + text in a lot of places?

@jrieken
Copy link
Member Author

jrieken commented Feb 8, 2022

@jrieken can we not use the checkmark before the "Apply"?

✔️

jrieken added a commit that referenced this issue Feb 8, 2022
@jrieken
Copy link
Member Author

jrieken commented Feb 11, 2022

Closing and tracking color tweaks as bug

@jrieken jrieken closed this as completed Feb 11, 2022
@jrieken jrieken added feature-request Request for new features or functionality verification-needed Verification of issue is requested labels Feb 23, 2022
@jrieken
Copy link
Member Author

jrieken commented Feb 23, 2022

To verify

  • in TS rename a symbol with preview
  • make sure the refactor preview window shows prominent accept/discard buttons

@isidorn
Copy link
Contributor

isidorn commented Feb 23, 2022

Works great -> verified
I filled a follow up issue #143736

@isidorn isidorn added the verified Verification succeeded label Feb 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality polish Cleanup and polish issue ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded workspace-edit
Projects
None yet
Development

No branches or pull requests

3 participants