Skip to content

Conversation

@Kurami32
Copy link
Collaborator

Description
Continuation of #1404
The original PR changed, so I think that is better open a new one also including the latest changes.

Summarizing:

  • A new status bar for the listing view which shows some info of the current folder where you are. Like folders, files, and their size (and the total size of the selected items - this is useful when you will do a download).
  • The size slider from the breadcrumbs moved to the status bar.
  • A "sub-view" for the gallery view, also placed in the status bar. (the "icons-view" in the previous PR)
  • Fixed typo in state.js and related files.

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

2025-10-23.17-31-39.mp4

@Kurami32 Kurami32 marked this pull request as draft October 23, 2025 22:36
@Kurami32 Kurami32 mentioned this pull request Oct 23, 2025
4 tasks
@Kurami32
Copy link
Collaborator Author

Kurami32 commented Oct 24, 2025

Now the slider also changes the size of the listing modes - And the main switch button only cycles between three modes.

2025-10-23.22-37-32.mp4

I think that I'm done! I'll update the translations.

@Kurami32
Copy link
Collaborator Author

I'm not sure why that test is failing, but anyways.

@Kurami32 Kurami32 marked this pull request as ready for review October 24, 2025 04:31
@gtsteffaniak
Copy link
Owner

gtsteffaniak commented Oct 24, 2025

did you check the test failure output?

image

It shows the console errors that are happening, these are real test failures from your changes. The steps are all succeeding -- but there are console errors that are occuring during the process which is why it failed.

see this test for steps to reproduce

test("navigate with hash in file name", async({ page, checkForErrors, context }) => {
await page.goto("/files/");
await expect(page).toHaveTitle("Graham's Filebrowser - Files - playwright-files");
await page.locator('a[aria-label="folder#hash"]').waitFor({ state: 'visible' });
await page.locator('a[aria-label="folder#hash"]').dblclick();
await expect(page).toHaveTitle("Graham's Filebrowser - Files - folder#hash");
await page.locator('a[aria-label="file#.sh"]').waitFor({ state: 'visible' });
await page.locator('a[aria-label="file#.sh"]').dblclick();
await expect(page).toHaveTitle("Graham's Filebrowser - Files - file#.sh");
await expect(page.locator('.topTitle')).toHaveText('file#.sh');
checkForErrors()
})

@Kurami32
Copy link
Collaborator Author

Kurami32 commented Oct 24, 2025

Finally, they now passed! Thanks for pointing where was the error.

const baseHeight = getters.viewMode() == "compact"
? 40 + (state.user.gallerySize * 2) // 40px to 56px - compact
: 50 + (state.user.gallerySize * 3); // 50px to 74px - list
document.documentElement.style.setProperty(
Copy link
Owner

@gtsteffaniak gtsteffaniak Oct 24, 2025

Choose a reason for hiding this comment

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

I think the precedent was set poorly here... and now that I see a bunch of document.documentElement.style.setProperty i don't like it.

Before it was just one, I could fix this later if you want but this is kinda bad practice, because we have this reactive vue application and setting style properties via vanilla javascript document queries.

not a blocker, but this really should be done here or components/files/listingItem.vue as part of the vue component.

I could clean this up later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry for that... please do it if it's not a hassle and if there no other issue 😅

@gtsteffaniak gtsteffaniak deleted the branch gtsteffaniak:dev/v0.8.11 October 28, 2025 17:30
@Kurami32
Copy link
Collaborator Author

Something happened?

@gtsteffaniak
Copy link
Owner

Oh I think because the dev branch deleted the PR also closed

@gtsteffaniak
Copy link
Owner

Just reopen against main and I will move to a dev branch when its ready. I am going to release v1.0.0 sometime this weekend and it will probably get merged in v1.1.0 following next week

@Kurami32
Copy link
Collaborator Author

Ah, I see, I'll do it then. 👍

@Kurami32 Kurami32 mentioned this pull request Oct 28, 2025
4 tasks
@desone1313
Copy link

desone1313 commented Dec 13, 2025 via email

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.

3 participants