-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Code Quality: Refactored command titles and descritption for clarity and consistency #17146
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
Changes from 8 commits
c59a6a2
27158c0
f69d8ae
8b6ddda
6c84a29
b18338f
6815ede
e98c33f
074ceb8
ea5921d
73ed7aa
15e6634
1561f94
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -130,7 +130,7 @@ | |||||
| <value>Copy path with quotes</value> | ||||||
| </data> | ||||||
| <data name="CopyItemPathWithQuotes" xml:space="preserve"> | ||||||
| <value>Copy selected item path with quotes</value> | ||||||
| <value>Copy item path with quotes</value> | ||||||
| </data> | ||||||
| <data name="Browse" xml:space="preserve"> | ||||||
| <value>Browse</value> | ||||||
|
|
@@ -232,7 +232,7 @@ | |||||
| <value>Continue where you left off</value> | ||||||
| </data> | ||||||
| <data name="SettingsOnStartupOpenANewTab.Content" xml:space="preserve"> | ||||||
| <value>Open a new tab</value> | ||||||
| <value>Open a new tab in the same window</value> | ||||||
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| </data> | ||||||
| <data name="SettingsOnStartupOpenASpecificPage.Content" xml:space="preserve"> | ||||||
| <value>Open a specific page or pages</value> | ||||||
|
|
@@ -1086,10 +1086,10 @@ | |||||
| <value>Toggle the details pane to view basic file properties</value> | ||||||
| </data> | ||||||
| <data name="ToggleInfoPane" xml:space="preserve"> | ||||||
| <value>Toggle the info pane</value> | ||||||
| <value>Toggle info pane</value> | ||||||
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| </data> | ||||||
| <data name="ToggleInfoPaneDescription" xml:space="preserve"> | ||||||
| <value>Toggle the info pane to view the detail/preview panes</value> | ||||||
| <value>Toggle the detail/preview panes</value> | ||||||
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| </data> | ||||||
| <data name="ToggleToolbar" xml:space="preserve"> | ||||||
| <value>Toggle toolbar</value> | ||||||
|
|
@@ -1917,7 +1917,7 @@ | |||||
| <value>Extract</value> | ||||||
| </data> | ||||||
| <data name="ExtractFiles" xml:space="preserve"> | ||||||
| <value>Extract files</value> | ||||||
| <value>Extract items</value> | ||||||
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| </data> | ||||||
| <data name="ExtractHere" xml:space="preserve"> | ||||||
| <value>Extract here</value> | ||||||
|
|
@@ -2208,7 +2208,7 @@ | |||||
| <value>Edit settings file</value> | ||||||
| </data> | ||||||
| <data name="EditSettingsFileDescription" xml:space="preserve"> | ||||||
| <value>Open settings file in your default editor</value> | ||||||
| <value>Open the configuration file in a text editor</value> | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you share info on the change from 'settings' to 'configuration'? I would also avoid the term 'a text editor' considering the file is not a text file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We switched from "settings" to "configuration" because "configuration file" is the more common and precise term in technical contexts. |
||||||
| </data> | ||||||
| <data name="ReleaseNotes" xml:space="preserve"> | ||||||
| <value>Release Notes</value> | ||||||
|
|
@@ -2229,7 +2229,7 @@ | |||||
| <value>Settings</value> | ||||||
| </data> | ||||||
| <data name="DoubleClickBlankSpaceToGoUp" xml:space="preserve"> | ||||||
| <value>Double click on a blank space to go up one directory</value> | ||||||
| <value>Double-click a blank space to navigate one level up the folder tree</value> | ||||||
|
||||||
| <value>Double-click a blank space to navigate one level up the folder tree</value> | |
| <value>Double-click blank space to navigate one level up the folder tree</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about compact overlay? 'Toggle' seems more appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is simply the title of the action, and for me, it is enough to put "Compact Overlay", because "Toggle" does not give more information about the state of the mode. Also, since I removed it from the title, I left it in the subtitle to avoid repeating "Toggle" in the title and subtitle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Compact Overlay' sounds like it's switching to Compact Overlay.
left it in the subtitle to avoid repeating "Toggle" in the title and subtitle.
As mentioned earlier, we don't have subtitles.
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is good in theory, but it would be best to revert for consistency with 'Exit compact overlay'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this is consistent with "switch to normal overlay mode"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My view is that both should be reverted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The app is either in "compact overlay mode' or not, 'normal overlay' isn't a mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the name of the mode when compact mode is not enabled? In this case, it is better to give it a name rather than saying "or not."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really have a name, the window is either in the Compact Overlay state or not. The closest I can think of is 'Regular mode' of 'Full mode', but that doesn't really provide a lot of context.
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using the term 'Rename' since that start of Files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind all these changes, as I explained earlier, is to avoid having an identical title and subtitle. So I understand that some sentences could be simplified, but otherwise, it would be pointless to have a title and subtitle containing the same sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, that said, we don't need duplicate strings in the resource file. If there are any duplicates, we can remove the extra one and reuse the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this any clearer than 'Select all items'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same thing. This is to add more precision because last time, my grandmother thought she had selected all the files from the root of her key and told me that it would be better to put "in the current view" (and to avoid repetition with the title so I thought that in passing I could add precision to the description)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we move forward with the change, it will need to update based on the context (eg. pane, column).
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the term 'application' helpful? Perhaps 'Open the settings page' would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more specific as it can be confusing with Windows Settings, as Windows is often mentioned in the app from what I've seen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to check that, do you remember where it is mentioned?
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Decrease' and 'Increase' seem to be more commonly used for these scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without context - the words "reduce size" go really well together. Just saying 😬
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jay-o-Way Navigate one level up in the folder tree vs Navigate one level up the folder tree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, tricky one, mostly looking at "navigate". That one is often used for websites and such. Simply saying "move one level up" sounds smoother. As for "in" or not, i would personally leave it out. Thinking of phrases like "move up a ladder".
In case of doubt, search MS Learn.
https://learn.microsoft.com/en-us/style-guide/a-z-word-list-term-collections/f/folder-folder-icon
https://learn.microsoft.com/en-us/style-guide/procedures-instructions/describing-interactions-with-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using the term 'navigate' for quite some time, so I'm not concerned about that part.
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Open' seems a bit redundant as you can't close a tab that's already closed 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By extent: "all" logically includes "current" tab 🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jay-o-Way true, however this is to provide contrast with a similar action that does not close the current tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This action isn't limited to the last tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but you won't open multiple tabs at once, only the last one. To reopen the last three, for example, run the action three times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, however the last closed tab was already reopened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but we can't open a tab that is already open
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Last' seems to imply that after using the action once, it won't be available unless the user closes another tab. 'Recently' isn't perfect, but I don't feel that 'Last' is better for this.
Lukiluc29 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Lukiluc29 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this comment for future reference (I need to think about this change);
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems slightly redundant...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought undo and redo were pretty common words.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, undo and redo are commonly used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <value>Revert the last file operation</value> | |
| <value>Undo the last file operation</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <value>Repeat the last file operation</value> | |
| <value>Redo the last file operation</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words "undo" 😆
yaira2 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for selected item
It's a little more complex. This action also works when no items are selected, and it also works when multiple items are selected. We can make the text contextual in the future, but it's out of scope for this PR. For now, I would keep the added 'the' but remove 'for selected item'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jay-o-Way any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instance is a bit redundant. I'm not against adding 'a', but it's worth considering that this will reset existing translations, and it might be best to just keep the current string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Horizonal' refers to the arrangement/orientation of the panes, we should avoid the term 'split' to prevent confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
selectedhelps provide a contrast with the action to copy the path of the current location.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. I removed the word "selected" because it wasn't useful to me. Also, the subtitle is there to properly describe the action, where it was correctly mentioned that it was only the selected item, and to shorten the title.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reemphasize, Actions don't have subtitles. This is the label used in tooltips (if we ever expose this option on the toolbar).