Dark #1269

Merged
btzr-io merged 4 commits from dark into master 2018-04-17 08:32:42 +02:00
btzr-io commented 2018-04-03 23:56:49 +02:00 (Migrated from github.com)

Changes

Fixes

  • Download percentage indicator overlay #1271
  • Fix dark theme: #1034
## Changes <img src=https://img00.deviantart.net/b04a/i/2016/092/3/9/join_the_dark_side_darth_vader_flag_by_osflag-d9xe8up.jpg > ## Fixes - Download percentage indicator overlay #1271 - Fix dark theme: #1034
btzr-io commented 2018-04-04 00:01:35 +02:00 (Migrated from github.com)
Fix dark theme: https://github.com/lbryio/lbry-app/issues/1034
neb-b (Migrated from github.com) requested changes 2018-04-04 03:08:24 +02:00
neb-b (Migrated from github.com) left a comment

Great start. Some of the dark mode colors changed with the new design.
Checkout http://lbry.zzk.fr to see the new colors 🙂

Great start. Some of the dark mode colors changed with the new design. Checkout http://lbry.zzk.fr to see the new colors 🙂
neb-b (Migrated from github.com) commented 2018-04-04 03:06:42 +02:00

The dashed/dotted should stay the same

The `dashed`/`dotted` should stay the same
btzr-io commented 2018-04-04 06:52:46 +02:00 (Migrated from github.com)

@seanyesmunt I added the new colors as an additional theme -> blueberry,
so now there are three themes: light, dark and blueberry

@seanyesmunt I added the new colors as an additional theme -> `blueberry`, so now there are three themes: `light, dark and blueberry`
btzr-io commented 2018-04-04 23:17:19 +02:00 (Migrated from github.com)

@seanyesmunt Ready for the next review

@seanyesmunt Ready for the next review
tzarebczan commented 2018-04-05 23:34:22 +02:00 (Migrated from github.com)

@btzr-io not sure if we want to maintain 3 themes going forward, but I'll let @seanyesmunt make the call.

Will give this a shot later tonight/tomorrow.

@btzr-io not sure if we want to maintain 3 themes going forward, but I'll let @seanyesmunt make the call. Will give this a shot later tonight/tomorrow.
neb-b (Migrated from github.com) requested changes 2018-04-10 22:11:24 +02:00
neb-b (Migrated from github.com) left a comment

@btzr-io Sorry it took so long for me to took at this.

I agree with Tom. I think at first we just want to release the "blueberry" theme (but call it dark theme).

The main issue I see is the buttons and the dropdowns (dropdown styling should probably be it's own issue).

The only button that is purple is the publish button. Everything else is still the same green. Also the forward/back needs hover styling.

@btzr-io Sorry it took so long for me to took at this. I agree with Tom. I think at first we just want to release the "blueberry" theme (but call it dark theme). The main issue I see is the buttons and the dropdowns (dropdown styling should probably be it's own issue). The only button that is purple is the publish button. Everything else is still the same green. Also the forward/back needs hover styling.
btzr-io commented 2018-04-12 01:45:30 +02:00 (Migrated from github.com)

@seanyesmunt Ok, it's done.

Dropdown styling should probably be it's own issue

This should be handle after merging: https://github.com/lbryio/lbry-app/issues/1317

@seanyesmunt Ok, it's done. > Dropdown styling should probably be it's own issue This should be handle after merging: https://github.com/lbryio/lbry-app/issues/1317
neb-b (Migrated from github.com) requested changes 2018-04-12 19:35:03 +02:00
neb-b (Migrated from github.com) left a comment

Left a few more comments.

Ideally we would have 0 !important uses in our styles. I know that might not be possible, but in most cases we can avoid them with class names or removing the styles that we want to override.

Left a few more comments. Ideally we would have 0 `!important` uses in our styles. I know that might not be possible, but in most cases we can avoid them with class names or removing the styles that we want to override.
neb-b (Migrated from github.com) commented 2018-04-12 19:25:02 +02:00

This shouldn't change. It should be a border on every side

This shouldn't change. It should be a border on every side
neb-b (Migrated from github.com) commented 2018-04-12 19:26:38 +02:00

This isn't the correct style. We shouldn't need to change anything here.

This isn't the correct style. We shouldn't need to change anything here.
@ -107,6 +110,7 @@ input {
}
input::placeholder {
color: var(--input-placeholder-color);
neb-b (Migrated from github.com) commented 2018-04-12 19:28:56 +02:00

We shouldn't need an !important here. If there is something that is overriding this style, we should remove that, or make this a more specific rule with classnames

We shouldn't need an `!important` here. If there is something that is overriding this style, we should remove that, or make this a more specific rule with classnames
neb-b (Migrated from github.com) commented 2018-04-12 19:31:00 +02:00

Do we need this? It doesn't look like it's doing anything. We should set any variables in vars.scss

Do we need this? It doesn't look like it's doing anything. We should set any variables in `vars.scss`
neb-b (Migrated from github.com) commented 2018-04-12 19:33:26 +02:00

Can we not use ::placeholder here?

Can we not use `::placeholder` here?
btzr-io (Migrated from github.com) reviewed 2018-04-12 20:23:11 +02:00
btzr-io (Migrated from github.com) commented 2018-04-12 20:23:11 +02:00

this overwrites the btn-alt background color inside modal, remove this line and open a modal with the dark theme selected, the button an background of the modal use the same color so is hard to see it.

this overwrites the `btn-alt` background color inside modal, remove this line and open a modal with the dark theme selected, the button an background of the modal use the same color so is hard to see it.
btzr-io (Migrated from github.com) reviewed 2018-04-12 20:53:50 +02:00
@ -40,7 +44,7 @@
right: 0;
bottom: 0;
btzr-io (Migrated from github.com) commented 2018-04-12 20:53:43 +02:00

@seanyesmunt Not sure if this is the right way to do this, but I don't have time to add a new class to all btn-alt used inside the modal component 😛

@seanyesmunt Not sure if this is the right way to do this, but I don't have time to add a new class to all `btn-alt` used inside the modal component :stuck_out_tongue:
btzr-io (Migrated from github.com) reviewed 2018-04-12 23:14:31 +02:00
btzr-io (Migrated from github.com) commented 2018-04-12 23:14:31 +02:00

Not sure why I did that 🙃

Not sure why I did that :upside_down_face:
neb-b (Migrated from github.com) reviewed 2018-04-17 00:18:06 +02:00
@ -40,7 +44,7 @@
right: 0;
bottom: 0;
neb-b (Migrated from github.com) commented 2018-04-17 00:18:06 +02:00

I think this is ok for now.

I think this is ok for now.
neb-b (Migrated from github.com) reviewed 2018-04-17 00:18:31 +02:00
@ -40,7 +44,7 @@
right: 0;
bottom: 0;
neb-b (Migrated from github.com) commented 2018-04-17 00:18:31 +02:00

Well, do modals in dark mode not use the regular alt button style?

Well, do modals in dark mode not use the regular alt button style?
neb-b commented 2018-04-17 00:18:35 +02:00 (Migrated from github.com)

I totally messed up the number of commits. Could you squash?

I think the only two issues I still see are that we lost the background-color for the text-copyable used in the address and file selector. And the "You have {lbc}" doesn't change color on hover.

I totally messed up the number of commits. Could you squash? I think the only two issues I still see are that we lost the background-color for the `text-copyable` used in the address and file selector. And the "You have {lbc}" doesn't change color on hover.
btzr-io (Migrated from github.com) reviewed 2018-04-17 03:45:45 +02:00
@ -40,7 +44,7 @@
right: 0;
bottom: 0;
btzr-io (Migrated from github.com) commented 2018-04-17 03:45:45 +02:00

@seanyesmunt yes but it's the same color as the background of the modal 🙃,
see: https://design.lbry.io

@seanyesmunt yes but it's the same color as the background of the modal :upside_down_face:, see: https://design.lbry.io
neb-b commented 2018-04-17 03:47:55 +02:00 (Migrated from github.com)

Ahh gotcha

Ahh gotcha
btzr-io commented 2018-04-17 04:26:08 +02:00 (Migrated from github.com)

Fix conflicts: a00f4eb

Fix conflicts: a00f4eb
neb-b (Migrated from github.com) approved these changes 2018-04-17 08:29:40 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#1269
No description provided.