-
Notifications
You must be signed in to change notification settings - Fork 0
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
style: SpoolSelectionDialog improvements #5
style: SpoolSelectionDialog improvements #5
Conversation
Signed-off-by: Pedro Lamas <[email protected]>
@@ -100,6 +109,9 @@ export default class AppDialog extends Vue { | |||
@Prop({ type: Boolean }) | |||
readonly noActions?: boolean | |||
|
|||
@Prop({ type: Boolean }) | |||
readonly titleShadow?: boolean |
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 a bit of a hack to get the shadow mode, but it works, so....
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.
Thanks. I've left a couple comments below:
small | ||
class="ms-1 my-1" |
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.
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 only reason I used those values is because we use them all over the app (ex: the cameras menu in camera card)
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.
Regarding the alignment, can you check what is the difference on the file explorer card?
It should have have same issue I think...
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.
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.
small | ||
class="ms-1 my-1" |
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.
small | |
class="ms-1 my-1" | |
class="mr-2" |
See above
Signed-off-by: Pedro Lamas <[email protected]>
I think I got all the margins covered now! As I said, the only reason I went with these values is that they match the values we are using elsewhere in the app, so I am just trying to keep consistency! |
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, thanks!
There are a couple of minor styling issues, but I thought it would be best practice to send you a PR to your PR instead of directly merging the changes! 🙂