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

Add custom decredmaterial button and clickable #630

Merged
merged 27 commits into from
Oct 4, 2021

Conversation

beansgum
Copy link
Contributor

@beansgum beansgum commented Sep 15, 2021

Closes #592
This pr implements a custom clickable highlight that allows custom color and border radius properties. A custom button layout is also added, it makes use of a custom clickable highlight and allows switching to a disabled state without directly changing the colors.

@Sirmorrison
Copy link
Contributor

Does this resolve #592 as well?

KIndly rebase.

@beansgum beansgum force-pushed the action_buttons branch 2 times, most recently from 8be6c9f to 68ada8c Compare September 15, 2021 18:56
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.

We will need to open an upstream issue to gio for the clickable widget drawInk(), as the issue with the clickable widget comes from there.

ui/decredmaterial/button.go Outdated Show resolved Hide resolved
ui/decredmaterial/button.go Outdated Show resolved Hide resolved
ui/page/components/nav_drawer.go Outdated Show resolved Hide resolved
ui/page/components/nav_drawer.go Outdated Show resolved Hide resolved
ui/page/components/nav_drawer.go Outdated Show resolved Hide resolved
ui/page/components/nav_drawer.go Outdated Show resolved Hide resolved
ui/page/components/nav_drawer.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@dreacot dreacot left a comment

Choose a reason for hiding this comment

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

The width of the selector on the dropdown should be full width there should be no margin on the selector also
Screenshot 2021-09-16 at 8 29 00 AM

@dreacot
Copy link
Collaborator

dreacot commented Sep 16, 2021

#624 seems to resolve #606 and #608 also

Copy link
Collaborator

@dreacot dreacot left a comment

Choose a reason for hiding this comment

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

The hover effect on the account selector should have a padding
Screenshot 2021-09-16 at 8 34 17 AM

@beansgum
Copy link
Contributor Author

This width of the hover highlight will be solved in another pull request as this one doesn't focus on modifying the layouts.

@beansgum
Copy link
Contributor Author

#624 seems to resolve #606 and #608 also

I think this should be merged first and #624 can focus and the layout width of the hover highlight since #624 is modifying individual layouts.

ui/decredmaterial/dropdown.go Outdated Show resolved Hide resolved
ui/decredmaterial/dropdown.go Outdated Show resolved Hide resolved
ui/page/components/account_selector.go Outdated Show resolved Hide resolved
dreacot
dreacot previously approved these changes Sep 16, 2021
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.

  • When the side and top nav buttons are hovered, the hover effect do not display.
  • Also, on the dropdown, the clickable even behaves unexpectedly as i would have to click twice for the dropdown to be displayed.
  • The create wallet button at the bottom right on the wallet page does not have the hover or click hightlight effect anymore

ezgif com-gif-maker(8)

  • When the loader icon is displayed, the cancel button should be hidden.

image

My recommendation will be to have this PR split into 2 different PRs.

This will enable us properly test the custom clickable widget and not block issue #595.

ui/decredmaterial/dropdown.go Outdated Show resolved Hide resolved
@dreacot
Copy link
Collaborator

dreacot commented Sep 24, 2021

Remove top and bottom padding on the dropdown
Screenshot from 2021-09-24 00-54-40

hovering on he wallet icon blends with the background
Screenshot from 2021-09-24 00-54-07

1 similar comment
@dreacot
Copy link
Collaborator

dreacot commented Sep 24, 2021

Remove top and bottom padding on the dropdown
Screenshot from 2021-09-24 00-54-40

hovering on he wallet icon blends with the background
Screenshot from 2021-09-24 00-54-07

@dreacot
Copy link
Collaborator

dreacot commented Sep 24, 2021

Hovering on some layouts in darkmode gives a bad contrast like this one
Screenshot from 2021-09-24 00-52-46

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.

image

Also, lets remove the top and bottom padding.
I understand you mentioned its from the design, however, the design did not implement the hover feature.
The padding is not nice on the UI so lets remove it.

This was what we had.
image

Comment on lines -103 to -106
if len(c.items[index].Text) > 12 {
txt = c.items[index].Text[:12] + "..."
}
c.items[0].label.Text = txt
Copy link
Contributor

Choose a reason for hiding this comment

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

This lines of code help truncate the wallet name should incase the wallet name is too long. Currently, we dont have a limit to the length of the wallet name, and to prevent the wallet name dropdown from being too long, it was given a maximum width.

This is the effect of removing the code.
image

@Sirmorrison
Copy link
Contributor

Sirmorrison commented Sep 24, 2021

The sync button font on the overview page looks bigger.
When the button is clicked, the reconnect widget looks broken

Master branch.
image

image

630

image

image

630
image
also, clicking on any of the links once, give an effect that looks like it was clicked twice.
ezgif com-gif-maker (1)

  • Same behavior can be seen on the receive page copy button.
  • On the account selector modal
  • Address and account switch button on the send page

The inactive/disable button color should be Theme.Color.InactiveGray the current color is too dark.
image

- Reduce tx details copy button text size and remove redundant creation
  of copy buttons.
- Fix clickable highlight bug
@beansgum
Copy link
Contributor Author

@Sirmorrison

@Sirmorrison
Copy link
Contributor

This behavior is still experienced
ezgif com-gif-maker (1)

  • Same behavior can be seen on the receive page copy button.
  • On the account selector modal
  • Address and account switch button on the send page.

also, from rescent review with Raedah, he has requested the the TxHash
image

should not be truncated, as long as we have enough space to display the full hash.

@Sirmorrison
Copy link
Contributor

Sirmorrison commented Sep 27, 2021

image
The create restore button on wallet page does not have the click effect.

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.

On the more page there is this extra shadow which is not present on master.
image

image

dreacot
dreacot previously approved these changes Sep 29, 2021
@dreacot dreacot merged commit 311b70a into planetdecred:master Oct 4, 2021
song50119 pushed a commit to song50119/godcr that referenced this pull request Apr 24, 2022
* Add custom button layout

- Remove all direct accesses of ButtonStyle clickable

* button: disable user interactions if disabled

* Add support for custom button highlight color

- add predefined styles for outline button

* Replace instances with explicit outline buttons style with predefined style

* Fix incorrect button background in dropdown & switch button

* Add clickable layout that supports custom background & radius

- apply custom clickable layout to clickable list

* Add Hover to decredmaterial Clickable

* Replace usages of Clickable with decredmaterial.Clickable

* Correctly handle button disabled states in all pages

* Wrap create wallet button in a white background

* Fix nav layout

* Add clickable hover to account selector modal

* Add clickable hover to dropdown widget

* Remove unused comments

* Add clickable to linear layout

- Fix nav drawer clickable layout

* Remove clickable parameter from outline button

* Remove clickable parameter from all buttons

* Fix dropdown layout clickable

* Cleanup dropdown widget code

* Add padding to account selector rows

* Fix highlight colors

* Cleanup more page row layout

* Truncate wallet name

* Fix reconnect button bug

- Reduce tx details copy button text size and remove redundant creation
  of copy buttons.
- Fix clickable highlight bug

* Fix click highlight color

* Fix wallet button highlight
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.

Remove Dark Highlight effect around create wallet button.
3 participants