-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
base: main
Are you sure you want to change the base?
Conversation
Normally, all the changes I had in mind have been implemented. It's up to you to tell me if everything suits you or if there are any changes to be made. |
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.
Pull Request Overview
This PR refactors various UI command titles and subtitles to improve clarity and consistency across the Files application.
- Updated labels and descriptions provide more context (e.g., specifying “in Windows Terminal,” “in the current view”).
- Changes span multiple command strings to align wording and improve user comprehension.
Comments suppressed due to low confidence (3)
src/Files.App/Strings/en-US/Resources.resw:2439
- Changing 'Cut' to 'Move' may alter the intended behavior; verify that the implementation aligns with this new terminology if the change is intentional.
<value>Move selected {0, plural, one {item} other {items}} to the clipboard</value>
src/Files.App/Strings/en-US/Resources.resw:3025
- Changing 'Undo' to 'Revert' might modify the intended semantics of the command; please confirm that this wording accurately reflects the functionality.
<value>Revert the last file operation</value>
src/Files.App/Strings/en-US/Resources.resw:3028
- Changing 'Redo' to 'Repeat' may lead to ambiguity in command behavior; verify that this change is consistent with the underlying functionality.
<value>Repeat the last file operation</value>
I am assuming we can't grab the name of the user's set default terminal, to make this string as accurate as we can. |
No, currently it is just a fixed string |
Just to clarify, actions have two strings. |
@@ -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> |
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.
selected
helps 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).
@@ -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> |
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.
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.
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 switched from "settings" to "configuration" because "configuration file" is the more common and precise term in technical contexts.
"Settings" usually refers to individual options or UI menus, while "configuration" covers the whole structure that defines how a system or program behaves.
Since we're talking about a file that controls the behavior of the application, "configuration file" felt more appropriate.
But I can revert "in a text editor" to return to "in your default editor".
@@ -2385,25 +2385,25 @@ | |||
<value>Preview popup</value> | |||
</data> | |||
<data name="ToggleCompactOverlay" xml:space="preserve"> | |||
<value>Toggle compact overlay</value> | |||
<value>Compact overlay</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.
</data> | ||
<data name="ExitCompactOverlayDescription" xml:space="preserve"> | ||
<value>Exit compact overlay mode</value> | ||
<value>Switch to normal overlay mode</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.
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.
</data> | ||
<data name="EnterCompactOverlayDescription" xml:space="preserve"> | ||
<value>Enter compact overlay mode</value> | ||
<value>Switch to compact overlay mode</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.
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.
@@ -2736,19 +2736,19 @@ | |||
<value>Close tabs other than selected tab</value> | |||
</data> | |||
<data name="CloseAllTabsDescription" xml:space="preserve"> | |||
<value>Close all tabs including the current tab</value> | |||
<value>Close all open tabs including the current tab</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.
'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.
</data> | ||
<data name="ReopenClosedTabDescription" xml:space="preserve"> | ||
<value>Reopen recently closed tab</value> | ||
<value>Reopen the last closed tab</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.
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.
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.
Just saying 😬
@@ -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> |
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.
"a blank space" makes it sound like "space" is a countable noun, like a space character.
<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> |
</data> | ||
<data name="LayoutDecreaseSizeDescription" xml:space="preserve"> | ||
<value>Decrease item size in the current view</value> | ||
<value>Reduce the icon or item size in view</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.
Without context - the words "reduce size" go really well together. Just saying 😬
@@ -2736,19 +2736,19 @@ | |||
<value>Close tabs other than selected tab</value> | |||
</data> | |||
<data name="CloseAllTabsDescription" xml:space="preserve"> | |||
<value>Close all tabs including the current tab</value> | |||
<value>Close all open tabs including the current tab</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.
By extent: "all" logically includes "current" tab 🤓
@@ -3016,16 +3016,16 @@ | |||
<value>Delete permanently</value> | |||
</data> | |||
<data name="DeleteItemPermanentlyDescription" xml:space="preserve"> | |||
<value>Delete selected {0, plural, one {item} other {items}} permanently</value> | |||
<value>Delete selected {0, plural, one {item} other {items}} permanently without recovery</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.
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
</data> | ||
<data name="PlayAllDescription" xml:space="preserve"> | ||
<value>Play the selected media files</value> | ||
</data> | ||
<data name="UndoDescription" xml:space="preserve"> | ||
<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.
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.
For strings that are modified but used elsewhere than in actions, can we create new strings so that they are individual and thus make the recommended changes without any problems? |
Yes |
@@ -3139,7 +3139,7 @@ | |||
<value>Properties</value> | |||
</data> | |||
<data name="OpenClassicProperties" xml:space="preserve"> | |||
<value>Explorer properties</value> | |||
<value>File Explorer properties</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.
As per @Jay-o-Way, this should start with 'Open'.
</data> | ||
<data name="AddHorizontalPane" xml:space="preserve"> | ||
<value>Horizontal pane</value> | ||
</data> | ||
<data name="AddHorizontalPaneDescription" xml:space="preserve"> | ||
<value>Add horizontal pane</value> | ||
<value>Add a horizontal split pane</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.
'Horizonal' refers to the arrangement/orientation of the panes, we should avoid the term 'split' to prevent confusion.
@@ -3788,7 +3788,7 @@ | |||
<value>Scroll to previous folder when navigating up</value> | |||
</data> | |||
<data name="NewWindowDescription" xml:space="preserve"> | |||
<value>Open new window</value> | |||
<value>Open a new window instance</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.
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.
@@ -3330,7 +3330,7 @@ | |||
<value>Command Palette</value> | |||
</data> | |||
<data name="OpenCommandPaletteDescription" xml:space="preserve"> | |||
<value>Open Command Palette in the OmniBar</value> | |||
<value>Open the Command Palette in the OmniBar</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.
@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
</data> | ||
<data name="OpenPropertiesDescription" xml:space="preserve"> | ||
<value>Open properties window</value> | ||
<value>Open the properties window for selected item</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.
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'
</data> | ||
<data name="RedoDescription" xml:space="preserve"> | ||
<value>Redo the last file operation</value> | ||
<value>Repeat 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" 😆
</data> | ||
<data name="PlayAllDescription" xml:space="preserve"> | ||
<value>Play the selected media files</value> | ||
</data> | ||
<data name="UndoDescription" xml:space="preserve"> | ||
<value>Undo the last file operation</value> | ||
<value>Revert 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>Revert the last file operation</value> | |
<value>Undo the last file operation</value> |
@@ -3016,16 +3016,16 @@ | |||
<value>Delete permanently</value> | |||
</data> | |||
<data name="DeleteItemPermanentlyDescription" xml:space="preserve"> | |||
<value>Delete selected {0, plural, one {item} other {items}} permanently</value> | |||
<value>Delete selected {0, plural, one {item} other {items}} without recovery</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.
@Lukiluc29 is there any reason to avoid the word 'permanently'? It accurately describes the operation and is consistent with the terminology in File Explorer.
@@ -3001,7 +3001,7 @@ | |||
<value>Show checkboxes when selecting items</value> | |||
</data> | |||
<data name="EditPathDescription" xml:space="preserve"> | |||
<value>Edit path in the OmniBar</value> | |||
<value>Edit the current folder path in the OmniBar</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.
Adding this comment for future reference (I need to think about this change);
<value>Switch to the previous tab</value> | ||
</data> | ||
<data name="NextTabDescription" xml:space="preserve"> | ||
<value>Move to the next tab</value> | ||
<value>Switch to the next tab</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.
I'm not against these two changes, but they seem to be very similar to the existing strings. Is it worth resetting the translations for essentially the same 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.
Actually go to is even better. "Switch" sounds like a setting or a control. https://learn.microsoft.com/en-us/style-guide/procedures-instructions/describing-interactions-with-ui
</data> | ||
<data name="NavigateUpDescription" xml:space="preserve"> | ||
<value>Navigate up one directory</value> | ||
<value>Navigate one level up in 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.
@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.
Resolved / Related Issues
To prevent extra work, all changes to the Files codebase must link to an approved issue marked as
Ready to build
. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.Steps used to test these changes
Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.