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

Convert all Toast alerts to Modals #1039

Closed
wants to merge 5 commits into from

Conversation

JustinBeBoy
Copy link
Contributor

@JustinBeBoy JustinBeBoy commented Aug 10, 2022

Resolve #1037

This PR custom success, error modals and change most of Toast alerts to modals

Screenshot
image

image

@JustinBeBoy JustinBeBoy marked this pull request as ready for review August 16, 2022 15:17
ui/page/wallet_settings_page.go Outdated Show resolved Hide resolved
ui/page/wallet_settings_page.go Outdated Show resolved Hide resolved
ui/values/localizable/en.go Show resolved Hide resolved
devchoplife
devchoplife previously approved these changes Aug 19, 2022
Copy link
Contributor

@devchoplife devchoplife left a comment

Choose a reason for hiding this comment

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

LGTM

@Fegor-Clinton
Copy link

This is a simple function that requires just a toggle switch on or off.
Also, the recent mockups doesn't contain any modal to confirm transaction notification success.
There is a ton of white space in this screenshot, it doesn't look like the initial pixels in which modals are designed with for GoDCR Version 2 was used here.

Copy link
Contributor

@Sirmorrison Sirmorrison left a comment

Choose a reason for hiding this comment

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

`bound([2405:4803:fe33:b7a0:a42e:4b85:2fbb:fd51]:19108): dial tcp [2405:4803:fe33:b7a0:a42e:4b85:2fbb:fd51]:19108: connect: no route to host
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0x4db30b1]

goroutine 36 [running]:
github.com/planetdecred/godcr/ui/page/components.(*DexServerSelector).startDexClient(0xc000397bf0)
/Users/sirmorrison/Desktop/go projects/godcr-gio/ui/page/components/dexserver_selector.go:89 +0x1b1
github.com/planetdecred/godcr/ui/page/components.(*DexServerSelector).Expose(...)
/Users/sirmorrison/Desktop/go projects/godcr-gio/ui/page/components/dexserver_selector.go:49
github.com/planetdecred/godcr/ui/page.(*WalletDexServerSelector).OnNavigatedTo(0xc0000acd80)
/Users/sirmorrison/Desktop/go projects/godcr-gio/ui/page/wallet_dex_selector_page.go:73 +0x7b
github.com/planetdecred/godcr/app.(*PageStack).Reset(0xc0000eaa50, {0xc0001dc2a0, 0x1, 0x1})
/Users/sirmorrison/Desktop/go projects/godcr-gio/app/pagestack.go:153 +0x112
github.com/planetdecred/godcr/app.(*SimpleWindowNavigator).ClearStackAndDisplay(0xc0000eaa80, {0x58ed270, 0xc0000acd80})
/Users/sirmorrison/Desktop/go projects/godcr-gio/app/window.go:84 +0x8f
github.com/planetdecred/godcr/ui/page.(*startPage).openWallets(0xc0020d7c00, {0x0, 0xc000472d00})
/Users/sirmorrison/Desktop/go projects/godcr-gio/ui/page/start_page.go:104 +0x16e
created by github.com/planetdecred/godcr/ui/page.(*startPage).OnNavigatedTo
/Users/sirmorrison/Desktop/go projects/godcr-gio/ui/page/start_page.go:59 +0xd1`

the app crash this log..

The Pr also need rebase.

And the Modal size used for the notification seems larger than the mockup as well.

@@ -12,7 +12,7 @@ type Icons struct {
ContentAdd, NavigationCheck, NavigationMore, ActionCheckCircle, ActionInfo, NavigationArrowBack,
NavigationArrowForward, ActionCheck, ChevronRight, NavigationCancel, NavMoreIcon,
ImageBrightness1, ContentClear, DropDownIcon, Cached, ContentRemove, ConcealIcon, RevealIcon,
SearchIcon, PlayIcon *widget.Icon
SearchIcon, PlayIcon, ErrorIcon *widget.Icon
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not needed as the icon used for the modal on the mockup already exist.

image

@@ -59,6 +59,7 @@ func (i *Icons) StandardMaterialIcons() *Icons {
i.RevealIcon = MustIcon(widget.NewIcon(icons.ActionVisibilityOff))
i.SearchIcon = MustIcon(widget.NewIcon(icons.ActionSearch))
i.PlayIcon = MustIcon(widget.NewIcon(icons.AVPlayArrow))
i.ErrorIcon = MustIcon(widget.NewIcon(icons.AlertError))
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed.

@@ -164,7 +164,10 @@ func (pg *VerifySeedPage) verifySeed() {
_, err := pg.WL.MultiWallet.VerifySeedForWallet(pg.wallet.ID, seed, []byte(password))
if err != nil {
if err.Error() == dcrlibwallet.ErrInvalid {
pg.Toast.NotifyError("Failed to verify. Please go through every word and try again.")
errModal := modal.NewErrorModal(pg.Load, "Failed to verify. Please go through every word and try again.", func(isChecked bool) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

lets localize all string variables used on the modal.

@Sirmorrison Sirmorrison marked this pull request as draft August 29, 2022 15:11
Copy link

github-actions bot commented Dec 7, 2023

No activity. Marking as stale

@github-actions github-actions bot added the stale label Dec 7, 2023
@github-actions github-actions bot closed this Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert all Toast alerts to Modals with an OK button to close.
4 participants