-
Notifications
You must be signed in to change notification settings - Fork 336
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
Enable the file opener #3350
Enable the file opener #3350
Conversation
@@ -172,7 +170,7 @@ class _CodeViewState extends State<CodeView> | |||
if (scriptRef == null) { | |||
return Center( | |||
child: Text( | |||
'No script selected', | |||
'Open a file: ⌘ P', | |||
style: theme.textTheme.subtitle1, |
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.
nice polish. One thing... we need to show the correct shortcut on both mac, linux, and windows so add a helper so we generate control
instead of ⌘
on linux and windows. @kenzieschmoll probably knows if we already have a helper like that. There are a few other places where we show keyboard shortcuts in the 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.
Yes we have a helper for this:
String describeKeys({bool isMacOS = false}) { |
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.
Oh nice! Switched to using that. One thing I noticed though is that the special character ⌘ isn't rendered properly when DevTools is running on the web.
This seems to be a known Flutter issue, and happens when Canvaskit is the renderer:
- [web] complex characters are rendered incorrectly flutter#79172
- [web] Some Unicode character not being shown flutter#77465
- [canvaskit] font renders missing glyph when text overflow is ellipsis flutter#76473
Would it make sense to use "cmd" instead of ⌘ on web until the rendering issue is fixed?
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, let's do that. Add a todo and link to the issue that will unblock this. Can you also file an issue on our repo to make sure we switch this back once the blockers are fixed? thanks
Enables the file opener dialog. Since it's now enabled, also changes the text when no file is opened to specify how to open a new file.