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.

Sign up for GitHub

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

Jump to bottom

Implement scrollbar to list #664

Merged
merged 9 commits into from
Oct 26, 2021
Merged

Implement scrollbar to list #664

merged 9 commits into from
Oct 26, 2021

Conversation

JustinBeBoy
Copy link
Contributor

@JustinBeBoy JustinBeBoy commented Oct 8, 2021 •
edited
Loading

Resolve #609

  • Upgrade gioui new version
  • Implement scrollbar for list
  • Update current component for gioui new version
  • Custom Icon

scroll1

scroll2

scroll3

@Sirmorrison
Copy link
Contributor

I dont think you will need those custom widgets you are adding since the recent updates to the gioui package has a scrollbar widget attached.
https://github.com/gioui/gio/blob/main/widget/list.go
Correct me if i am wrong but i believe this is what we want.

@JustinBeBoy JustinBeBoy force-pushed the scrollbar branch 3 times, most recently from b0b51dd to 5f08b5d Compare October 12, 2021 03:42
@JustinBeBoy JustinBeBoy changed the title add custom list with scroll view Upgrade Gioui new version and implement scrollbar to list Oct 12, 2021
@JustinBeBoy JustinBeBoy marked this pull request as ready for review October 12, 2021 06:30
@Sirmorrison
Copy link
Contributor

Kindly rebase this Pr as the gioui library has been updated to a more recent version.

also, from the first screenshot your dropped, the scrollbar widget does not align with the page content area.. that the the area that is to scroll.
image

The second screenshot aligns well to the scroll area, however, it is too close to the content card.. there should be some padding in-between.
image

The 3rd screenshot has it scroll bar inside the content area. It should be outside like the others.
image

@JustinBeBoy JustinBeBoy force-pushed the scrollbar branch 2 times, most recently from c4bbb07 to 983a0c5 Compare October 18, 2021 14:53
@Sirmorrison Sirmorrison changed the title Upgrade Gioui new version and implement scrollbar to list Implement scrollbar to list Oct 18, 2021
Sirmorrison
Sirmorrison suggested changes Oct 18, 2021
View reviewed changes
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.

Kindly pay attention to reviews on the icons size parameter.

ui/decredmaterial/icon.go Outdated Show resolved Hide resolved
ui/decredmaterial/icon.go Outdated Show resolved Hide resolved
ui/decredmaterial/icon.go Outdated Show resolved Hide resolved
ui/modal/info_modal.go Outdated Show resolved Hide resolved
ui/modal/info_modal_layouts.go Outdated Show resolved Hide resolved
ui/page/send/send_confirm_modal.go Outdated Show resolved Hide resolved
ui/page/send/send_confirm_modal.go Outdated Show resolved Hide resolved
ui/page/settings_page.go Outdated Show resolved Hide resolved
ui/page/tickets/utils.go Outdated Show resolved Hide resolved
ui/page/tickets/vsp_selector.go Outdated Show resolved Hide resolved
Sirmorrison
Sirmorrison previously requested changes Oct 21, 2021
View reviewed changes
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.

Scrollbar not visible on the Overview page.
image

No scroll bar on tx details page.
image

No scroll bar on wallet page when all the wallets are expanded.
image

Not so sure why this icon is on the left of your page instead of the right.
image

The scroll padding on the proposals page is not uniform as the against tx and ticket pages.
image

Proposal details content area does not have a scroll bar.
image

Setting page does not have a scroll bar.
image

  • Wallet Log Page and statistics page do not also have scroll bars.

The idea of this PR is to add scroll bars to all pages that scrolls. Meaning any page that has the list layout.

ui/decredmaterial/clickable_list.go Outdated Show resolved Hide resolved
ui/page/proposal/proposals_page.go Outdated Show resolved Hide resolved
ui/page/wallet_page.go Outdated Show resolved Hide resolved
ui/page/wallet_page.go Outdated Show resolved Hide resolved
ui/decredmaterial/clickable_list.go Outdated Show resolved Hide resolved
ui/page/seedbackup/verify_seed.go Outdated Show resolved Hide resolved
ui/page/tickets/overview.go Outdated Show resolved Hide resolved
@Sirmorrison Sirmorrison force-pushed the scrollbar branch 2 times, most recently from 5fd8ec8 to 607b54f Compare October 22, 2021 15:09
JustinBeBoy and others added 7 commits October 25, 2021 14:51
@JustinBeBoy @Sirmorrison
add custom list with scroll view
8868c21
@JustinBeBoy @Sirmorrison
update gio version and refactor code with new gioui version
b27f5b3
@JustinBeBoy @Sirmorrison
rebase and fix scroll bar in page used
04a1aef
@JustinBeBoy @Sirmorrison
refactor Size in Icon
df9e835
@Sirmorrison
fix add wallet menu btn displacement
f1022f8 
- add list_scroll widget to decred material
- rename HideScroll to IsHiddenScrollBer
@Sirmorrison
Add scroll bar to overview page, wallet page,
e855342 
- wallet seed backup page and fix seed view page scroll bug
- add scrollbar to tx details and account details pages
- add scroll to ticket page, remove ticket gridlayout
@Sirmorrison
Fix see all icon size bug
84b5d5b 
- add scrollbar to proposals details page,
- license page, log page, statistics page
@Sirmorrison Sirmorrison force-pushed the scrollbar branch 2 times, most recently from 2cfec7a to 94bc3c4 Compare October 25, 2021 17:00
@Sirmorrison
Remove scrollbar from clickable list widget.
28f0178 
- clean up tx page alignment
- ensure uniform scrollbar padding
- rebase PR and clean up code
dreacot
dreacot reviewed Oct 25, 2021
View reviewed changes
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 all tickets page shows a long scrollbar, but the remainder of the page is blank
Screenshot 2021-10-25 at 6 21 20 PM

This page should have a scrollbar
Screenshot 2021-10-25 at 6 23 54 PM

This modal should have a scrollbar
Screenshot 2021-10-25 at 6 24 43 PM

This page is scrolled to the bottom but the scrollbar isn't
Screenshot 2021-10-25 at 6 26 37 PM

dreacot
dreacot previously approved these changes Oct 26, 2021
View reviewed changes
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.

LGTM

ui/page/transactions_page.go Show resolved Hide resolved
@Sirmorrison
Add scrollbar to send and receive page
bfb14e6 
- Add scrollbar to modal widget
- fix wallet and ticket page scrollbar bug
dreacot
dreacot approved these changes Oct 26, 2021
View reviewed changes
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.

LGTM

@dreacot dreacot merged commit bc5fc39 into planetdecred:master Oct 26, 2021
song50119 pushed a commit to song50119/godcr that referenced this pull request Apr 24, 2022
@JustinBeBoy @Sirmorrison
Implement scrollbar to list (planetdecred#664)
331f447 
* add custom list with scroll view

* update gio version and refactor code with new gioui version

* rebase and fix scroll bar in page used

* refactor Size in Icon

* fix add wallet menu btn displacement
- add list_scroll widget to decred material
- rename HideScroll to IsHiddenScrollBer

* Add scroll bar to overview page, wallet page,
- wallet seed backup page and fix seed view page scroll bug
- add scrollbar to tx details and account details pages
- add scroll to ticket page, remove ticket gridlayout

* Fix see all icon size bug
- add scrollbar to proposals details page,
- license page, log page, statistics page

* Remove scrollbar from clickable list widget.
- clean up tx page alignment
- ensure uniform scrollbar padding
- rebase PR and clean up code

* Add scrollbar to send and receive page
- Add scrollbar to modal widget
- fix wallet and ticket page scrollbar bug

Co-authored-by: sirmorrison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dreacot dreacot dreacot approved these changes

@Sirmorrison Sirmorrison Sirmorrison left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add slidder to pages that scroll.
3 participants
@JustinBeBoy @Sirmorrison @dreacot