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

Dark #1269

Merged
merged 4 commits into from Apr 17, 2018

Conversation

Projects
None yet
4 participants
@btzr-io
Collaborator

btzr-io commented Apr 3, 2018

Changes

Fixes

  • Download percentage indicator overlay #1271
  • Fix dark theme: #1034

@btzr-io btzr-io requested a review from seanyesmunt Apr 3, 2018

@lbry-bot lbry-bot assigned seanyesmunt and unassigned liamcardenas Apr 3, 2018

@btzr-io btzr-io requested a review from tzarebczan Apr 3, 2018

@btzr-io

This comment has been minimized.

Show comment
Hide comment
@btzr-io

btzr-io Apr 3, 2018

Collaborator

Fix dark theme: #1034

Collaborator

btzr-io commented Apr 3, 2018

Fix dark theme: #1034

@seanyesmunt

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

Show outdated Hide outdated src/renderer/scss/_gui.scss
@btzr-io

This comment has been minimized.

Show comment
Hide comment
@btzr-io

btzr-io Apr 4, 2018

Collaborator

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

Collaborator

btzr-io commented Apr 4, 2018

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

@btzr-io

This comment has been minimized.

Show comment
Hide comment
@btzr-io

btzr-io Apr 4, 2018

Collaborator

@seanyesmunt Ready for the next review

Collaborator

btzr-io commented Apr 4, 2018

@seanyesmunt Ready for the next review

@tzarebczan

This comment has been minimized.

Show comment
Hide comment
@tzarebczan

tzarebczan Apr 5, 2018

Member

@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.

Member

tzarebczan commented Apr 5, 2018

@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.

@seanyesmunt

@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

This comment has been minimized.

Show comment
Hide comment
@btzr-io

btzr-io Apr 11, 2018

Collaborator

@seanyesmunt Ok, it's done.

Dropdown styling should probably be it's own issue

This should be handle after merging: #1317

Collaborator

btzr-io commented Apr 11, 2018

@seanyesmunt Ok, it's done.

Dropdown styling should probably be it's own issue

This should be handle after merging: #1317

@seanyesmunt

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.

Show outdated Hide outdated src/renderer/scss/_gui.scss
Show outdated Hide outdated src/renderer/scss/_gui.scss
Show outdated Hide outdated src/renderer/scss/_gui.scss
Show outdated Hide outdated src/renderer/scss/component/_modal.scss
Show outdated Hide outdated src/renderer/scss/component/_search.scss
.btn.btn--alt {
// Set modal buttons bg color
background-color: var(--modal-btn-bg-color);
}

This comment has been minimized.

@btzr-io

btzr-io Apr 12, 2018

Collaborator

@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 😛

@btzr-io

btzr-io Apr 12, 2018

Collaborator

@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 😛

This comment has been minimized.

@seanyesmunt

seanyesmunt Apr 16, 2018

Member

I think this is ok for now.

@seanyesmunt

seanyesmunt Apr 16, 2018

Member

I think this is ok for now.

This comment has been minimized.

@seanyesmunt

seanyesmunt Apr 16, 2018

Member

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

@seanyesmunt

seanyesmunt Apr 16, 2018

Member

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

This comment has been minimized.

@btzr-io

btzr-io Apr 17, 2018

Collaborator

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

@btzr-io

btzr-io Apr 17, 2018

Collaborator

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

@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Apr 16, 2018

Member

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.

Member

seanyesmunt commented Apr 16, 2018

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.

@seanyesmunt

This comment has been minimized.

Show comment
Hide comment
@seanyesmunt

seanyesmunt Apr 17, 2018

Member
Member

seanyesmunt commented Apr 17, 2018

@btzr-io

This comment has been minimized.

Show comment
Hide comment
@btzr-io

btzr-io Apr 17, 2018

Collaborator

Fix conflicts: a00f4eb

Collaborator

btzr-io commented Apr 17, 2018

Fix conflicts: a00f4eb

@btzr-io btzr-io removed the request for review from tzarebczan Apr 17, 2018

@seanyesmunt seanyesmunt merged commit baff4e3 into master Apr 17, 2018

0 of 3 checks passed

Codacy/PR Quality Review Hang in there, Codacy is reviewing your Pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@btzr-io btzr-io deleted the dark branch Apr 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment