-
Notifications
You must be signed in to change notification settings - Fork 326
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 "About app" modal #9833
Add "About app" modal #9833
Conversation
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.
LGTM, except a few issues
} | ||
|
||
return ( | ||
<Modal centered className="bg-dim"> |
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.
Please stop copypasting between components. We have Dialog component, use it instead
const textContainerRef = React.useRef<HTMLTableSectionElement | null>(null) | ||
|
||
const doCopy = () => { | ||
const textContainer = textContainerRef.current |
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 think it'd be easier to understand if we build a string from existing variables(window.versionInfo and so forth), instead of trying to extract the information from html.
There's also 1 missing thing: maybe we want to allow users to get the versions dialog on every screen(even on the login screen)? not only on the dashboard |
QA #1 ✅ Passed with a few moments to fix:
![]() |
re 2: yeah. the intention was for chat to eventually work when logged out, but i guess it's not particularly useful as it is currently |
Pull Request Description
cmd
+?
Keybind Does Not Show Enso Version #9433Important Notes
None
Screenshots
In browser (both cloud entrypoint, and accessing
localhost:8080
when the desktop app is running):On Electron desktop app:
On authentication pages:
(NOTE: Chat is present, but currently disabled as intended for users with the
isEnabled
flag set tofalse
. While it would be easy to change this to use the same chat API as the website, this is a design decision that is out of scope for this PR.)Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.