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.

Sign up for GitHub

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

Jump to bottom

Use detailed text for reload button in extension editor #59074

Merged
merged 3 commits into from
Sep 21, 2018
Merged

Use detailed text for reload button in extension editor #59074

merged 3 commits into from
Sep 21, 2018

Conversation

ramya-rao-a
Copy link
Contributor

This PR is to update the text on the Reload button in the extension editor to include hint on the reload is needed. This resolves the discussion in #56028

@ramya-rao-a ramya-rao-a self-assigned this Sep 21, 2018
octref
octref suggested changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@octref octref left a comment

Choose a reason for hiding this comment

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

Reload to deactivate -> Reload to Deactivate

We use TitleCase for buttons in other places.

@octref
Copy link
Contributor

octref commented Sep 21, 2018

We use TitleCase for context menus items:

image

image

The button's text look very out of place:

image

@ramya-rao-a
Copy link
Contributor Author

Fixed the casing.
One catch is that, since we re-use the same text for the tooltip in the extension viewlet, we will have title casing in the tooltip as well. Having separate texts for the tooltip and label is not worth the effort.

octref
octref approved these changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@octref octref left a comment

Choose a reason for hiding this comment

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

Lookin' good

@ramya-rao-a ramya-rao-a merged commit 2492847 into master Sep 21, 2018
@ramya-rao-a ramya-rao-a deleted the ramyar/reload branch September 21, 2018 17:31
@RMacfarlane
Copy link
Contributor

@ramya-rao-a Looks like there are tests that expect the old casing:
https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/extensions/test/electron-browser/extensionsActions.test.ts#L1032

@ramya-rao-a
Copy link
Contributor Author

Fixed.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@octref octref octref approved these changes

Assignees

@ramya-rao-a ramya-rao-a

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ramya-rao-a @octref @RMacfarlane