Skip to content

Fix Slider widget properties#1770

Open
RagibHasin wants to merge 7 commits intolinebender:mainfrom
RagibHasin:fix-slider-props
Open

Fix Slider widget properties#1770
RagibHasin wants to merge 7 commits intolinebender:mainfrom
RagibHasin:fix-slider-props

Conversation

@RagibHasin
Copy link
Copy Markdown
Contributor

@RagibHasin RagibHasin commented Apr 25, 2026

Follow up of #1691

@RagibHasin
Copy link
Copy Markdown
Contributor Author

RagibHasin commented Apr 25, 2026

It seems that our image diffing algorithm did not/does not pick up the change in appearance of this and some other widgets.

We should use a high contrasting background color/transparent background color for test harness to reliably catch these.

@RagibHasin RagibHasin force-pushed the fix-slider-props branch 2 times, most recently from 262d4f7 to 0fe2a85 Compare April 27, 2026 16:59
@PoignardAzur
Copy link
Copy Markdown
Contributor

PoignardAzur commented Apr 28, 2026

It seems that our image diffing algorithm did not/does not pick up the change in appearance of this and some other widgets.

Wait, what?

@RagibHasin
Copy link
Copy Markdown
Contributor Author

RagibHasin commented Apr 28, 2026

It seems that our image diffing algorithm did not/does not pick up the change in appearance of this and some other widgets.

Wait, what?

Yes, it's wild. Although to be fair the difference is very minimal with very low contrast with background.

PoignardAzur
PoignardAzur reviewed Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@PoignardAzur PoignardAzur 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 the PR!

I appreciate simplifying the widget code, just not sure about the behavior changes.

Comment thread masonry/src/properties/slider.rs
Comment thread masonry/screenshots/slider_drag_initial_at_25.png
@xStrom
Copy link
Copy Markdown
Member

xStrom commented Apr 28, 2026

It seems that our image diffing algorithm did not/does not pick up the change in appearance of this and some other widgets.

Which image diffs were not picked up? The one that you updated anyway here or something else? I would like to take a look at the specific test case that doesn't get detected.

@RagibHasin
Copy link
Copy Markdown
Contributor Author

RagibHasin commented Apr 28, 2026

I would like to take a look at the specific test case that doesn't get detected.

I would see if I can dig those up.

@RagibHasin RagibHasin requested a review from PoignardAzur April 29, 2026 17:33
@RagibHasin
Copy link
Copy Markdown
Contributor Author

RagibHasin commented May 7, 2026

It sits ready for review, @xStrom .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xStrom xStrom Awaiting requested review from xStrom

@PoignardAzur PoignardAzur Awaiting requested review from PoignardAzur

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RagibHasin @PoignardAzur @xStrom