Skip to content

Conversation

@Kurami32
Copy link
Collaborator

Description
#1425

According to the contributing guide, A PR should contain:

  • A clear description of why it was opened.
  • A short title that best describes the change.
  • Must pass unit and integration tests, which can be run checked locally prior to opening a PR.
  • Any additional details for functionality not covered by tests.

Additional Details

@gtsteffaniak gtsteffaniak changed the base branch from main to dev/v1.1.0 November 2, 2025 21:41
@gtsteffaniak
Copy link
Owner

gtsteffaniak commented Nov 2, 2025

This looks great! just a few things I think need cleanup:

  1. The positioning doesn't appear anchored to the bottom, should be fixed positioning with bottom: 0, but tall windows show a separation.
  2. mobile styling doesn't go entirely to the sides
  3. The choice of icon being a dynamic and nearly identical to the top-right might be a confusing choice - a bit too much cognitive load to keep track of two different states at opposite sides of the screen. It should just be a simple toggle button that doesn't change, maybe change_circle material icon?
image image

@Kurami32
Copy link
Collaborator Author

Kurami32 commented Nov 3, 2025

Ah, that seems easy to fix.
But for 1 is odd, because I used a fixed position with bottom 0, I will check that again.
And for 3, sure, I'll change it to that icon.

@Kurami32 Kurami32 changed the base branch from dev/v1.1.0 to main November 3, 2025 00:41
@Kurami32
Copy link
Collaborator Author

Kurami32 commented Nov 3, 2025

Ah, was in v1.1.0 already 😅

@Kurami32 Kurami32 changed the base branch from main to dev/v1.1.0 November 3, 2025 00:43
@Kurami32
Copy link
Collaborator Author

Kurami32 commented Nov 3, 2025

Now I think that the gallery view on mobile is a lot better :)

Screenshot_20251103-020420_FileBrowser Quantum.jpg

Screenshot_20251103-020415_FileBrowser Quantum.jpg

@gtsteffaniak
Copy link
Owner

gtsteffaniak commented Nov 3, 2025

looks great now! I feel fine merging this as-is but it may get tweaked... so I would rather iron out the changes here before merging.

I still don't quite think the toggle button feels right. I think we discussed maybe a toggle button for whether the aspect ratio is respected. But having the toggle switch between compact and list view and gallery and new gallery view doesn't seem worth it. I think we discussed having the views change based on the size scale? What do you think about doing that instead, remove the toggle button and instead base it on size ? if size = 0 or 1 (out of 9) , use the minified view (compact list and new compact gallery view)

@Kurami32
Copy link
Collaborator Author

Kurami32 commented Nov 3, 2025

Well for me this is finished 😅

I was thinking on do your suggestion but that may break all... Basically the two modes in gallery are two different modes, that's why I called one "icons view" and the other is still called gallery, which uses (very) bigger items on desktop, meanwhile on mobile I tried to mimic the look of native mobile gallery app.

If I do that probably will need a complete re-write (again) of all, the columns calculation, the CSS... and probably don't will behave well because there will be very sudden changes in size because of the size levels. (And I'm not sure how to do it properly).

However I think that I can do it for the list views as they are literally the same but without the indicator of folders and file at the top.

But thinking about this- probably is better a setting/preference of group items instead, applicable for all the view modes and delete one of the list view modes, but that's maybe another separate feature.

@gtsteffaniak
Copy link
Owner

ok no worries -- this is 99% of what it should be so I think its perfectly fine to merge and it will be another week or so before release, im sure it will get tweaked in dev branches.

@gtsteffaniak gtsteffaniak merged commit fb8dcd8 into gtsteffaniak:dev/v1.1.0 Nov 3, 2025
17 checks passed
@Kurami32 Kurami32 deleted the feat/status-bar branch November 3, 2025 19:38
@Kurami32 Kurami32 restored the feat/status-bar branch November 4, 2025 01:31
@Kurami32 Kurami32 deleted the feat/status-bar branch November 4, 2025 01:33
@gtsteffaniak gtsteffaniak mentioned this pull request Nov 19, 2025
gtsteffaniak pushed a commit that referenced this pull request Nov 19, 2025
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