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

Improve ScrollArea #325

Merged
merged 6 commits into from
Oct 29, 2024
Merged

Improve ScrollArea #325

merged 6 commits into from
Oct 29, 2024

Conversation

veger
Copy link
Collaborator

@veger veger commented Oct 23, 2024

This PR adds a collection of small improvements and preparation for freezing the first column (tab/page name column) of the SummaryView ScrollArea. This is working almost fully working and I am using it locally all the time: the only left-over issue is that the the scrollbars are disappeared. (so the UX is a little off 😉 )
I pushed this 'freeze-first-summary-column' branch with this fix to my own fork, so it is be more publicly available, until I (or someone) is able to fix the missing scrollbars, so we can actually merge!


I have these changes for ages in my local repo, and I do think these are suitable on their own to be committed. It will help me (re)fixing them on style changes or other code changes.

Each change is a separate commit (as I think we should keep them), so it is fairly easy if we do not like one of them (then I'll move back to my freeze-first-summary-column where most of them are required to make this feature work.

For testing I opened up YAFC and clicked around a bit, all still seem to be working/rendering the same, except:

  • empty Summary rows (now) properly render (8930a9c)
  • scrolling still works (without the fallback, see b6e04c2 and 52491b1)

@veger veger requested a review from shpaass as a code owner October 23, 2024 19:45
@veger veger force-pushed the improve_scrollarea branch 2 times, most recently from 4a91fab to 52491b1 Compare October 23, 2024 19:53
@shpaass
Copy link
Owner

shpaass commented Oct 23, 2024

I'm thinking about merging this one after the transition to Factorio 2.0 is done with bugfixes.
That is because we have critical bugs right now, and we don't want to add more uncertainty until we fix that.
Is it fine with you?

@shpaass
Copy link
Owner

shpaass commented Oct 29, 2024

Added a space after commit prefixes

@shpaass
Copy link
Owner

shpaass commented Oct 29, 2024

Thank you for the improvements!

Required when a parent needs to calculate exact row height.
This way it is dependent on the actual icon sizes, instead of the
hardcoded sizes.
This prevents 'compressing' empty rows in the SummaryView
It is interesting for parents, in order to layout the other components in the parent context.
…ly consumed

Removes the 'fallback' of horizontal scrolling without CTRL
when the vertical scrollbar is not present. It is confusing and breaking
underlying panels (as they won't receive (expected) events.
@shpaass
Copy link
Owner

shpaass commented Oct 29, 2024

Rebased on fresh master

@shpaass shpaass merged commit 4a68750 into shpaass:master Oct 29, 2024
1 check passed
@veger veger deleted the improve_scrollarea branch October 29, 2024 12:02
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