-
Notifications
You must be signed in to change notification settings - Fork 18
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
add strings for #2391 #19
Conversation
en/messages.json
Outdated
@@ -252,6 +252,10 @@ | |||
} | |||
} | |||
}, | |||
"openInNoContainer": { | |||
"message": "Open in No Container", |
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 feels a bit weird. Would it work with Open in tab without container
, or Open in standard tab
? Also, I realized recently that buttons should use sentence case, according to Firefox copy guidelines. I wouldn't go and fix them all, but maybe we can fix these two?
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 used the same wording of the context menu when we right-click on new tab button in the tabbar. I agree it sounds weird.
Looking at the wording in other parts of the UI (context menu when right clicking a tab), we could change both buttons to have something like:
[Open in New Tab] [Open in New containerName Container Tab]
What do you think of this solution?
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 used the same wording of the context menu when we right-click on new tab button in the tabbar. I agree it sounds weird.
Right, but that's stand-alone on top of a list of containers, I don't think it's relevant for this button.
[Open in New Tab] [Open in New containerName Container Tab]
This would work for me (again, sentence case though). In the end, even if "Open in new tab" might not be 100% clear, it's still a significant improvement on the current behavior.
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.
On second thoughts though: it opens in the same tab, so it might be misleading? Open without container
and Open in $containerName$ container
might be a better option.
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, it does open a new tab for the containerized one that replace the regular one. Granted, it isn't visible for the user.
What's bugging me is the negative "without", "no", etc. I guess we could simply use:
[Open in current tab] OR [Open in Work container 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.
[Open in current tab] OR [Open in Work container tab]
Works for me 👍🏼
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 change the case also in the previous string (same button for container)? No need to update the ID, that's internal style for English.
D
Done ;) |
We can merge this when multi-account-containers#2391 is merged.