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

web/settings: Fix quick action selection #32258

Closed
wants to merge 4 commits into from
Closed

web/settings: Fix quick action selection #32258

wants to merge 4 commits into from

Conversation

yowayb
Copy link
Contributor

@yowayb yowayb commented Mar 7, 2022

Test plan

Note: no component snapshots (https://kentcdodds.com/blog/effective-snapshot-testing)

  • Before/after screenshots of a few combinations of Settings Actions

Questions

  • Would a Percy visual test be appropriate?

- Replaced `setProperty()` calls with `modify()`
@yowayb yowayb changed the title - Fixed #2756 Fixes #2756 Mar 7, 2022
@cla-bot
Copy link

cla-bot bot commented Mar 7, 2022

We require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

The reviewers' guide for accepting the contribution.

@cla-bot
Copy link

cla-bot bot commented Mar 7, 2022

We require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

The reviewers' guide for accepting the contribution.

@cla-bot
Copy link

cla-bot bot commented Mar 8, 2022

We require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

The reviewers' guide for accepting the contribution.

@yowayb
Copy link
Contributor Author

yowayb commented Mar 8, 2022

Screenshots

Click "Search: show # before/after lines":

Before After (the selection of the 3 is hard to see)
image image

Click "Search: show # before/after lines", then "Add search snippet":

Before After (no change)
image image

Click "Add search snippet", then "Search: show # before/after lines" :

Before After (the selection of the 3 is hard to see)
image image

Click "Search: show # before/after lines", then "Add quick link" (note: this was fixed by changing the placeholder text, because this Action specifies a selection. Without this fix, the entire array after "quicklinks": would be selected):

Before After
image image

@yowayb yowayb marked this pull request as ready for review March 9, 2022 14:26
@yowayb
Copy link
Contributor Author

yowayb commented Mar 9, 2022

@sourcegraph/contribution-reviewers

@yowayb yowayb changed the title Fixes #2756 web settings: Fix quick action selection Mar 10, 2022
@yowayb yowayb changed the title web settings: Fix quick action selection web/settings: Fix quick action selection Mar 10, 2022
@yowayb yowayb closed this by deleting the head repository Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix selection after running a settings quick action
1 participant