Conversation
|
@daiyam sorry for the late response I was quite busy. This PR is very helpful, can you fix the conflict for it please? Thank you. |
|
@ZeroX-DG I will try |
|
@ZeroX-DG I've fixed the conflicts and rewrite the styles of new themed components. |
|
Very nice, I'll review this soon 😄 |
ZeroX-DG
left a comment
There was a problem hiding this comment.
This is a very cool PR, just need a bit adjustment I think. Please tell me if I'm wrong 😄
| .root | ||
| .slider | ||
| border-left 1px solid $ui-solarized-dark-borderColor | ||
| for theme in 'dark' 'dracula' 'solarized-dark' |
There was a problem hiding this comment.
Why don't you include 'dark' 'dracula' 'solarized-dark' to $themes variable too? If there's a reason for this, can you explain it to me?
There was a problem hiding this comment.
Ideally, 'dark' 'dracula' 'solarized-dark' and 'default' 'white' should be in the variable $themes. But since I don't want to create new variables for the styles, I didn't merge all the styles.
| transition 0.15s | ||
| background-color alpha(#f8f8f2, 10%) | ||
| color $ui-dracula-text-color | ||
| for theme in 'dracula' |
There was a problem hiding this comment.
This loop only have 'dracula' can you also move this to the $themes variable for the loop bellow?
| background-color get-theme-var(theme, 'button-backgroundColor') | ||
| color get-theme-var(theme, 'text-color') | ||
|
|
||
| .menu-button--active |
There was a problem hiding this comment.
Can we merge .menu-button-start--active, .menu-button-active and .menu-button-trash--active?
| .menu-button--active | |
| .menu-button--active, .menu-button-star--active, .menu-button-trash--active |
There was a problem hiding this comment.
Ya, we could be see for white and dark, those buttons have different colors. So what to do? Create new variables or leave it like this?
There was a problem hiding this comment.
I would prefer to create new variables. After all we need to keep the consistency for this. Do you think you can add them?
There was a problem hiding this comment.
It will take some time and with BoostNote.next, I'm not sure if it's worth.
There was a problem hiding this comment.
True, that's why I'm kinda hesitated. Let's leave it, I'll see if I can refactor it later.
|
|
||
| body[data-theme="dracula"] | ||
| .root | ||
| color $ui-dracula-text-color |
There was a problem hiding this comment.
I think you miss this line in the rewrite? Or we don't need it? 😄
There was a problem hiding this comment.
Ya, there is an issue with the template.
But there is another issue, the theme solarized-dark is different from the master.
The master use the white theme whereas the current one use solarized-dark colors.
…corresponding themes
|
@ZeroX-DG I've fixed the issue with the titles |
ZeroX-DG
left a comment
There was a problem hiding this comment.
Very cool 😄 I'll approve this. Thank you for this PR 👍
Description
This change adds the Nord theme as UI theme.
I've updated the UI theme selection box:
I've also streamlined how to manage the UI themes in Javascript and Stylus.
Issue fixed
Type of changes
Checklist: