Skip to content

feat(Widgets): FadeBin#1354

Merged
GeopJr merged 9 commits intomainfrom
feat/widgets/fadebin
Mar 18, 2025
Merged

feat(Widgets): FadeBin#1354
GeopJr merged 9 commits intomainfrom
feat/widgets/fadebin

Conversation

@GeopJr
Copy link
Owner

@GeopJr GeopJr commented Mar 7, 2025

One of these days I'll make libgeopjr or something because it's a shame for all these custom widgets to not be used in other apps.

Anyway, faded view. Not sure if we should go forward with this but it's configurable enough to be able to auto-reveal with a setting or similar.

Screencast.From.2025-03-07.16-58-05.mp4

It could also come with a configurable max-height but for now it's a constant.

@GeopJr
Copy link
Owner Author

GeopJr commented Mar 12, 2025

@bugaevc quick question, would this have any impact on hitting the fast path?

FadeBin is HEIGHT_FOR_WIDTH unless there's a child, which then uses the child's mode. In statuses, there's always a child, the markupview.

The rest is mainly setting the min (<=) and nat (vertical) sizes to 300. Allocation passes to the child.

Copy link
Contributor

@bugaevc bugaevc left a comment

Choose a reason for hiding this comment

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

Done a quick review, will do a second round when you fix these 🙂

Layout wise, yes, this shouldn't break the fast paths.

I'd expect the fade/size to animate when expanded, like Gtk.Revealer does. Do you not want that?

@GeopJr
Copy link
Owner Author

GeopJr commented Mar 12, 2025

Thanks for the reviews! If you conclude that this is not feasible in a performant way, I'm okay with dropping it.

@GeopJr GeopJr marked this pull request as draft March 12, 2025 12:19
@bugaevc
Copy link
Contributor

bugaevc commented Mar 12, 2025

I do think it is both feasible and interesting, please keep going 🙂

GeopJr and others added 2 commits March 12, 2025 14:46
 Co-authored-by: Sergey Bugaev <[email protected]>
Co-authored-by: Sergey Bugaev <[email protected]>
@GeopJr
Copy link
Owner Author

GeopJr commented Mar 12, 2025

I'd expect the fade/size to animate when expanded, like Gtk.Revealer does. Do you not want that?

It might be too much for very long posts but I'll see about it. If we go for it, I'd probably guess:

  • snapshot whole child no fade
  • overflow hidden
  • set min/nat to int.max (minimum * animation.value, MAX_HEIGHT);

@bugaevc
Copy link
Contributor

bugaevc commented Mar 12, 2025

set min/nat to int.max (minimum * animation.value, MAX_HEIGHT);

It's a lot more complex than that. Basically, you want to animate the height limit from MAX_HEIGHT up to "child's actual height". Again, I would have to think for (a much longer) while about the correct way here. It's probably best if you don't block this PR on the animation, we can add it later.

@GeopJr
Copy link
Owner Author

GeopJr commented Mar 12, 2025

we can add it later

👍

This PR is not done anyway, it was an initial implementation. I still need to add settings and polish the UI, strings etc.

@bugaevc
Copy link
Contributor

bugaevc commented Mar 14, 2025

Here's a working sketch of the animation
https://gitlab.gnome.org/-/snippets/6864

@GeopJr
Copy link
Owner Author

GeopJr commented Mar 17, 2025

Thanks! Is there a specific reason on doing the animation manually instead of using AdwTimedAnimation?

@bugaevc
Copy link
Contributor

bugaevc commented Mar 17, 2025

Ah, sure, feel free to use that. I was focusing on the implementation of measure & size allocate, not on everything else.

Co-authored-by: Sergey Bugaev <[email protected]>
@GeopJr
Copy link
Owner Author

GeopJr commented Mar 17, 2025

I don't think I skipped anything, but the main difference is that FadeBin's API needs a way to expand without animation for cases like users having collapsing long posts off and pre-expanding them when opening a thread, so there's the reveal property and the reveal_animated (and hide_animated) functions. When the animation ends, the reveal property inverts.

Screencast.From.2025-03-17.21-17-17.mp4

@bugaevc
Copy link
Contributor

bugaevc commented Mar 17, 2025

  • Does it work — does it animate nicely? Can you record a video? 😃 Ideally, I should build this branch, slow down the animation, and verify that nothing about the layout invariants breaks.
  • Not sure if the LOTS thing causes GSK to render a huge offscreen texture, or if it already does optimizations based on bounds etc.

@bugaevc
Copy link
Contributor

bugaevc commented Mar 17, 2025

I see, cool 😄

@GeopJr
Copy link
Owner Author

GeopJr commented Mar 17, 2025

Not sure if the LOTS thing causes GSK to render a huge offscreen texture, or if it already does optimizations based on bounds etc.

I wonder if extreme edge cases will have any impact (100k+ character long posts) 🤔 Will test it out later.

Thanks again for everything! 🙇

@GeopJr GeopJr marked this pull request as ready for review March 18, 2025 18:33
@GeopJr GeopJr merged commit cc70e74 into main Mar 18, 2025
4 of 5 checks passed
@GeopJr GeopJr deleted the feat/widgets/fadebin branch March 18, 2025 18:33
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.

2 participants