-
-
Notifications
You must be signed in to change notification settings - Fork 480
[General] Improvements in install dialog #1129
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
Conversation
--dialog-gap: 24px; | ||
} | ||
|
||
.Dialog__element { |
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.
Anything against camelCase
? ahahah
we use camel case on the other variables and CSS classnames would be good to be consistent :)
--dialog-gap: 24px; | ||
} | ||
|
||
.Dialog__element { |
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.
Anything against camelCase
? ahahah
we use camel case on the other variables and CSS classnames would be good to be consistent :)
@@ -24,10 +38,14 @@ import { | |||
Path, | |||
Runner | |||
} from 'src/types' | |||
import Dialog from '../../../../components/UI/Dialog' |
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 have relative imports configured on Heroic so you can do it like:
import Dialog from 'src/components/UI/Dialog'
and so on.
@@ -24,10 +38,14 @@ import { | |||
Path, | |||
Runner | |||
} from 'src/types' | |||
import Dialog from '../../../../components/UI/Dialog' |
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 have relative imports configured on Heroic so you can do it like:
import Dialog from 'src/components/UI/Dialog'
and so on.
@@ -24,10 +38,14 @@ import { | |||
Path, | |||
Runner | |||
} from 'src/types' | |||
import Dialog from '../../../../components/UI/Dialog' | |||
import DialogContent from '../../../../components/UI/Dialog/DialogContent' |
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 believe you could have an index.ts file that imports all Dialog
children's (like we are doing with sidebar, for instance) and then just import all of them on the same line:
import {DialogContent, DialogFooter, DialogHeader} from 'src/components/UI/Dialog/components'
@@ -24,10 +38,14 @@ import { | |||
Path, | |||
Runner | |||
} from 'src/types' | |||
import Dialog from '../../../../components/UI/Dialog' | |||
import DialogContent from '../../../../components/UI/Dialog/DialogContent' |
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 believe you could have an index.ts
file that imports all Dialog
children's (like we are doing with sidebar, for instance) and then just import all of them on the same line:
import {DialogContent, DialogFooter, DialogHeader} from 'src/components/UI/Dialog/components'
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 it okay, now?
Tested a bit here and looks good. |
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.
Looks pretty good now. Thanks for that!
Improvements in install dialog:
<dialog>
element for handling dialogs which comes with some advantages, like focus trap and esc to closePreview:




Use the following Checklist if you have changed something on the Backend or Frontend: