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

✨ Support full windows path navigation #431

Merged
merged 7 commits into from
Sep 7, 2024
Merged

Conversation

JMicheli
Copy link
Collaborator

@JMicheli JMicheli commented Sep 1, 2024

This pull request adds support for navigating to Windows drives other than the C:\ drive by making use of several (unsafe - but I followed the documentation and linked it in the comments) syscalls to collect the available drives and their names,

Here is what would be seen previously on a Windows machine (note the disabled back button):

image

This is what you see now:

image

The only lingering issue is that when going back to the "root" view from a drive, the path shown in the Input component doesn't seem to revert to a blank. If I can work this out, I'll fix it (though I welcome suggestions - my frontend is weak).

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
apps/server/src/routers/api/v1/filesystem.rs 0.00% 7 Missing ⚠️
Files with missing lines Coverage Δ
apps/server/src/routers/api/v1/filesystem.rs 0.00% <0.00%> (ø)

Copy link
Collaborator

@aaronleopold aaronleopold left a comment

Choose a reason for hiding this comment

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

I think this looks great! I'll try to find time to test it this week on my laptop to ensure it doesn't regress on unix systems.

The only lingering issue is that when going back to the "root" view from a drive, the path shown in the Input component doesn't seem to revert to a blank. If I can work this out, I'll fix it (though I welcome suggestions - my frontend is weak).

So this is one of the older bits of mostly unchanged code in the repo, so offhand I can't say I'm familiar with how I originally put it together. I'll take a look when I load these changes up locally as I mentioned above. A guess could be the input needs to have an empty string value instead of undefined as I currently have it

@JMicheli JMicheli changed the title Support full windows path navigation ✨ Support full windows path navigation Sep 2, 2024
@aaronleopold
Copy link
Collaborator

Hey, wanted to check in and say I haven't forgotten about this. I've been caught up in the performance bug/issues and haven't had a chance to take a look yet. I'll aim to give it a proper review over the weekend

@aaronleopold
Copy link
Collaborator

A guess could be the input needs to have an empty string value instead of undefined as I currently have it

This was it 🙂

Copy link
Collaborator

@aaronleopold aaronleopold left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@aaronleopold aaronleopold enabled auto-merge (squash) September 7, 2024 22:39
@aaronleopold aaronleopold merged commit ee78d6b into develop Sep 7, 2024
6 checks passed
@aaronleopold aaronleopold deleted the jm/windows-dirs branch September 12, 2024 00:30
@aaronleopold aaronleopold mentioned this pull request Oct 11, 2024
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