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

feat(ui): added a size slider for list display mode #1688

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MavikBow
Copy link

@MavikBow MavikBow commented Jan 31, 2025

Copies a feature added to the aniyomi fork. Here's a visual demonstration of what this adds.

Added a size slider for the List Display Mode. It uses the same size variable as Grids do to determine size. However, the size of the List gets interpreted not as <number> of columns per row, but rather as <number> of entries of the list visible on the screen at once.

The style of the List was changed: the cover is of Book resolution to match the Grids and for better visibility.

Other two lists (browse list and migration list) don't have editable size, but neither do browse grids, so it's fine. Instead they now have a predetermined size of 76.dp (was 56.dp previously) and are visually changed to match the library list style. Same for the default size of the list.

The size slider also has two additional text fields added, which would need translation (though i assume that's automated).

The max line restriction for titles was removed, allowing for longer names to be fully visible, given the List is set to a big enough size.

closes #1687

@MavikBow
Copy link
Author

I need to work on this a little more, so I'm closing this temporarily.

@MavikBow MavikBow closed this Jan 31, 2025
@AntsyLich
Copy link
Member

Just mark it as draft

@MavikBow MavikBow reopened this Jan 31, 2025
@MavikBow MavikBow marked this pull request as draft January 31, 2025 19:29
@MavikBow
Copy link
Author

MavikBow commented Feb 6, 2025

Sorry for the delay. I'm waiting when these changes will be merged the aniyomi fork. Because if my code would need corrections, I wouldn't want to bother both repositories to fixup them.

@AntsyLich AntsyLich marked this pull request as ready for review February 7, 2025 03:16
@AntsyLich AntsyLich marked this pull request as draft February 7, 2025 03:16
@AntsyLich
Copy link
Member

Added a size slider for the List Display Mode. It uses the same size variable as Grids do to determine size. However, the size of the List gets interpreted not as of columns per row, but rather as of entries of the list visible on the screen at once.

I don't really like this approach

@MavikBow
Copy link
Author

MavikBow commented Feb 7, 2025

Well... could you go into more detail as to why you don't like it? The way I see it, the grids scale as a number per row, so imho that's the logical way to implement this for the list.
If I went for a hardcoded size for all slider steps, several issues would emerge:
1) might look bad on some devices,
2) may needs further work if slider sees changes.

I guess in an ideal world the list would have to be rewritten as a one column grid to be able to utilize this Adaptive Cell feature (I hope I'm correct, I'm quite new to jetpack compose), but I don't need all lists to be affected, just the library and browse ones; nor does anyone have the time to do that.

What approach do you like? I'll see what I can propose. I still want to able to scale the list, that's why I'm here.

@AntsyLich
Copy link
Member

I don't have any ideas at the moment but one thing I can say is that this should be a separate setting and you get to decrease the number of visible items not increase it.

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.

Add a size slider for the List Display Mode
2 participants