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

Introduce a new theme size name for rounded scrollbar corners #4839

Merged
merged 12 commits into from
Jun 22, 2024
Merged

Introduce a new theme size name for rounded scrollbar corners #4839

merged 12 commits into from
Jun 22, 2024

Conversation

dweymouth
Copy link
Contributor

@dweymouth dweymouth commented May 10, 2024 •
edited by andydotxyz
Loading

Description:

Fixes #4360

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@dweymouth dweymouth changed the base branch from master to develop May 10, 2024 15:35
@andydotxyz
Copy link
Member

Seems more like it should be a new theme lookup - scrollbars aren't really a selection type I don't think.

@dweymouth dweymouth changed the title Round scrollbar using theme.SelectionRadiusSize Introduce a new theme size name for rounded scrollbar corners May 11, 2024
@dweymouth dweymouth added this to the "Elgin" release, early 2024 milestone May 11, 2024
Jacalz
Jacalz previously approved these changes May 18, 2024
View reviewed changes
Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

I know this isn't completed but I just want to give my thumbs up to say that this looks great and would be a great inclusion for v2.5.0 👍

andydotxyz
andydotxyz reviewed May 22, 2024
View reviewed changes
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks for adding this, just a question about the size

theme/theme.go Outdated Show resolved Hide resolved
@andydotxyz
Copy link
Member

This looks slick - do you need any assistance getting the tests in order?

@dweymouth
Copy link
Contributor Author

Sure, feel free to give it a go - I think it's just test images but I have been solely working on M2 mac recently

@andydotxyz
Copy link
Member

Sent dropbox link over Slack

@dweymouth dweymouth marked this pull request as ready for review June 18, 2024 14:25
dweymouth
dweymouth commented Jun 21, 2024
View reviewed changes
theme/size.go Outdated Show resolved Hide resolved
@coveralls
Copy link

Coverage Status

coverage: 65.879% (-0.001%) from 65.88%
when pulling 79cb0d9 on dweymouth:round-scrollbar
into 1ce4224 on fyne-io:develop.

@andydotxyz
Copy link
Member

That's all the tests files updated post-merge and golden data refreshed too... Ready to land if someone else can approve.

andydotxyz
andydotxyz previously approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Approved, but I feel biased so maybe @Jacalz could too?

@coveralls
Copy link

Coverage Status

coverage: 65.889% (+0.009%) from 65.88%
when pulling 28c067f on dweymouth:round-scrollbar
into 1ce4224 on fyne-io:develop.

@dweymouth dweymouth merged commit c3215ee into fyne-io:develop Jun 22, 2024
12 checks passed
@dweymouth dweymouth deleted the round-scrollbar branch June 22, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andydotxyz andydotxyz andydotxyz approved these changes

@Jacalz Jacalz Jacalz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
"Elgin" release, early 2024
Development

Successfully merging this pull request may close these issues.

Scroll bar should have rounded corners, at least in its expanded state
4 participants
@dweymouth @andydotxyz @coveralls @Jacalz