Skip to content
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

Implement dark mode #401

Merged
merged 7 commits into from May 25, 2021
Merged

Implement dark mode #401

merged 7 commits into from May 25, 2021

Conversation

tuhalang
Copy link
Contributor

@tuhalang tuhalang commented May 8, 2021

Resolve #399

  • Create switch button on setting page
  • Switch light mode or dark mode when clicking the switch button

ui/decredmaterial/label.go Outdated Show resolved Hide resolved
ui/compontents.go Outdated Show resolved Hide resolved
ui/compontents.go Outdated Show resolved Hide resolved
ui/compontents.go Outdated Show resolved Hide resolved
@oshorefueled
Copy link
Contributor

The dark mode doesn't seem to be working well. This is what I get on my end when I launch the app
Screenshot 2021-05-11 at 10 53 04

@oshorefueled
Copy link
Contributor

I get a crash with this log when I run the app.

goroutine 1 [running, locked to thread]:
github.com/planetdecred/dcrlibwallet.(*MultiWallet).ReadUserConfigValue(0x0, 0x4e43e0c, 0xc, 0x4bf9400, 0xc0008ec059, 0x4e34600, 0x1)
/Users/klinsmann/go/pkg/mod/github.com/planetdecred/dcrlibwallet@v1.5.3-0.20210224132742-5d0bc5e13370/multiwallet_config.go:64 +0x44
github.com/planetdecred/dcrlibwallet.(*MultiWallet).ReadBoolConfigValueForKey(...)
/Users/klinsmann/go/pkg/mod/github.com/planetdecred/dcrlibwallet@v1.5.3-0.20210224132742-5d0bc5e13370/multiwallet_config.go:110
github.com/planetdecred/godcr/wallet.(*Wallet).ReadBoolConfigValueForKey(0xc00079a640, 0x4e43e0c, 0xc, 0x40800000ffb5513f)
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/wallet/commands.go:773 +0x71
github.com/planetdecred/godcr/ui.(*Window).OverviewPage(0xc000457200, 0xc00049a6e0, 0xc00079a640, 0xc00010c000, 0xc000457310, 0xc0004572a0, 0xc0003e6360, 0xc00066cfa0, 0xc00066cff0, 0xc00066d040, ...)
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/ui/overview_page.go:90 +0xbb
github.com/planetdecred/godcr/ui.(*Window).loadPage(0xc000457200, 0xc00049a6e0, 0xc00079a640, 0xc00010c000, 0xc000457310, 0xc0004572a0, 0xc0003e6360, 0xc00066cfa0, 0xc00066cff0, 0xc00066d040, ...)
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/ui/page.go:356 +0x11f
github.com/planetdecred/godcr/ui.(*Window).addPages(0xc000457200, 0xc000651dd0)
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/ui/page.go:350 +0x6749
github.com/planetdecred/godcr/ui.CreateWindow(0xc00079a640, 0xc000651dd0, 0xc00079a8c0, 0x1, 0x1, 0xc00049c600, 0x0, 0x1, 0x0)
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/ui/window.go:122 +0x809
main.main()
/Users/klinsmann/go/src/github.com/raedahgroup/godcr/main.go:121 +0x5e8

@oshorefueled
Copy link
Contributor

The address text on the receive page isn't visible
Screenshot 2021-05-14 at 22 05 41

Modals don't seem to be affected by the dark mode
Screenshot 2021-05-14 at 22 05 55

Copy link
Contributor

@beansgum beansgum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some texts are not readable

Screenshot 2021-05-21 at 08 07 18
Screenshot 2021-05-21 at 08 08 13

- Remove function LableColor
- Add dark mode for ticket page
- Sometimes, the application is crashed because the wallet hasn't loaded
- Checking the wallet is not nil before read the config properties
- Fix darkmode for modals
- Fix darkmode for receive address
- Fix some texts are not readable when switch darkmode
@@ -162,7 +163,7 @@ func (win *Window) OverviewPage(c pageCommon) layout.Widget {
pg.cachedIcon = c.icons.cached

return func(gtx C) D {
pg.Handler(gtx, c)
pg.Handler(gtx, c, win)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to pass the window struct to the handler method. It seems you're trying to call the reload method from the handler after toggling the dark mode option. Is there a reason you need to re-initialize the icons and pages after toggling the dark mode option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i need re-initialize pages after toggling the dark mode option.

ui/page.go Outdated
Comment on lines 385 to 387
func (win *Window) reloadPage(common pageCommon) {
win.loadPage(common.icons)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a redundant wrapper for the loadPage method.

if win.wallet != nil {
isDarkModeOn := win.wallet.ReadBoolConfigValueForKey("isDarkModeOn")
if isDarkModeOn != win.theme.DarkMode {
win.theme.SwitchDarkMode(isDarkModeOn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this method called in the overview and settings page. Shouldn't it be called just once from the settings page?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I called it in the settings page to handle the button click event dark mode. In the overview page because it is the first page loaded when app startup, i can read config from the wallet in this page and switch to the dark mode.

@@ -71,7 +79,8 @@ func (page pageCommon) layoutUSDBalance(gtx layout.Context) layout.Dimensions {
return border.Layout(gtx, func(gtx C) D {
return padding.Layout(gtx, func(gtx C) D {
amountDCRtoUSDString := formatUSDBalance(page.printer, page.amountDCRtoUSD)
return page.theme.Label(values.TextSize14, amountDCRtoUSDString).Layout(gtx)
return page.theme.Body2(amountDCRtoUSDString).Layout(gtx)
//return page.theme.LabelColor(values.TextSize14, amountDCRtoUSDString, page.theme.Color.DeepBlue).Layout(gtx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove redundant comments

Comment on lines +38 to +42
label := page.theme.Label(values.TextSize20, mainText)
if isSwitchColor {
label.Color = page.theme.Color.DeepBlue
}
return label.Layout(gtx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the colour for the text being done specifically here, shouldn't it be affected like other text colours in the app?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the text in here alway display black color independent dark or light mode
image

@oshorefueled oshorefueled dismissed Sirmorrison’s stale review May 25, 2021 14:28

request has been resolved

@oshorefueled oshorefueled merged commit 809285f into planetdecred:master May 25, 2021
song50119 pushed a commit to song50119/godcr that referenced this pull request May 29, 2021
song50119 pushed a commit to song50119/godcr that referenced this pull request Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement dark mode
4 participants