Remove request for non native recording directory #3009
Conversation
One reason could have been documentation. Using the Qt dialog means that each OS gets the same Look and Feel when choosing a file/directory from Jamulus. Such usage in the documentation is then clear to the user. Using the OS-native one means we need to document each OS separately, so the user is hand-held (noting that GUI server users are likely less technical). The up-side is that they're also likely more familiar with the OS-native widget. Another reason could have been testing: one can make assumptions like "if I've tested the workflow on Linux with the Qt widget, it'll work on Windows with the Qt widget". That may be an invalid assumption using the OS-native widget. For example, choosing a file and choosing a directory may use different widgets - I think I hit some issue there at some point. Qt uses the same widget for both. |
|
Ok. Probably we'll only know more if we test it on all OS |
This is done to allow write access to a recording directory outside of the macOS sandbox
ann0see
force-pushed
the
remove-non-native-recordingdir
branch
from
bb3a4a9
to
9b5e1b2
Compare
|
Code looks okay, I'll wait for test feedback to approve. |
|
macOS unsigned artifact from this build works. |
|
@emlynmac can you/me push the changes to your repo and test if the recording directory can now be set outside the sandbox? |
|
I've now signed the build with a self signed certificate via #2944 here: https://github.com/ann0see/jamulus/actions/runs/4193064214 and I can save files (= Recordings) outside of the Sandbox (I tested the Downloads folder). Linux (Debian also doesn't show any issue; storing to /tmp - there's no sandbox enabled on Debian, so it shouldn't matter at all). Windows 11 also seems good. |
I've sync'd. |
|
@emlynmac Thanks. Unfortunately, I only see that the main branch got built. Could you please git checkout my changes from this PR and build them? |
|
The build seems to do what it should. Thanks! |
|
Do we have example screen shots yet? |
|
It's just the normal OS file/directory dialog. I can take some pictures tomorrow |
|
I've no idea what that looks like :). Also, we need examples from all supported platforms for "Open file" and "Open directory" (also, in case there are differences, with different permission requests). |
|
Sorry for the delay. @pljones here are the screenshots: macOS Open Directory:
Default compact view for selecting file:
Extended view for selecting file:
Debian 12 GNOME Open directory:
Selecting file:
Windows 11 Open directory:
Selecting file:
|
Thanks! Really useful as I've currently only access to a Chromebook, so couldn't even do Windows ones easily. |
|
@pljones do you consider this as mergable? |
|
Yes, feel free to merge. |
Short description of changes
Removes the request for a non native directory selection box on all OS for the recording directory and the persistence file. I hope this doesn't introduce any bad side effects.
This is done to allow write access to a recording directory outside of the macOS sandbox for the Server.
CHANGELOG: Server: Use native file selection dialog for recording directory and persistence file to allow read and write access outside of the macOS sandbox.
Context: Fixes an issue?
Related to #2131
Does this change need documentation? What needs to be documented and how?
Probably yes?
Status of this Pull Request
To be tested with a signed build. I'd like to have an ok from @emlynmac to build on his repo and a confirmation why we chose to use the Qt file dialog instead of the native OS one by @pljones
What is missing until this pull request can be merged?
Internal clarification and testing: Waiting on OK from emlynmac to build on his repo.
Checklist