Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Use newer Windows file-picker #52

Closed
jt15s opened this issue Sep 4, 2021 · 28 comments
Closed

Use newer Windows file-picker #52

jt15s opened this issue Sep 4, 2021 · 28 comments

Comments

@jt15s
Copy link

jt15s commented Sep 4, 2021

I've noticed Audacium uses the older style Windows file-picker when selecting files, which have older icons, UI and aren't theme-compatible with the rest of the OS. More frustratingly, it has a different layout to File Explorer, which makes it a bit harder to find files.
image

Most apps use a newer file-picker, which opens a mini-version of File Explorer. Hence it uses the familiar File Explorer layout, is theme-compatible and also has the latest UI and icons provided by Windows. The following example is what happens when one tries to upload a file to Discord:
image

Thanks in advance!
@AnErrupTion
Copy link
Contributor

This might be a wxWidgets restriction, but I'm not sure. I'll see.

@AnErrupTion
Copy link
Contributor

Nervermind, it looks like there's a file dialog wrapper for Windows, macOS and Linux and the Windows one is based on older APIs. It would take quite some time to rewrite that wrapper.

@generic-pers0n
Copy link

Hello guys!

This will be very interesting since I'm the developer of Saucedacity. Regardless, I hope to contribute something to this issue. ;)

Although I have never taken a look at Audacium's code base, it is derived from Audacity. As Saucedacity is derived from Audacity also, I believe we should have a lot of things in common.

@AnErrupTion was right to an extent; this is a wxWidgets restriction. However, Audacity uses its own FileDialog (and, correspondingly, FileDialogBase), which are derived from wxWidgets classes on the source level (i.e., the actual source files were imported into Audacity and modified) as I have discovered. (At least, this is true with Saucedacity; it should be the same with Audacium). It also appears that these classes are modified from wxWidgets 3.0.2.

What I have done in Saucedacity is that I have basically updated these classes from wxWdigets 3.1.5, which appear to use the newer Windows file picker. It didn't take much, admittedly, but I expect there to be a catch uncaught on my part to this. (If such arises, feel free to discuss this with me).

So, while there is a solution to this issue now, I would like to discuss this a bit further. I have yet to incorporate this fix into Audacium and test it, but I hope that we can get this fixed sometime in Audacium sometime soon (with the side effect of this being fixed in Saucedacity).

@AnErrupTion
Copy link
Contributor

Another idea would be to just remove Audacium's modified FileDialog, and use wxFileDialog and a simple wrapper (like AudaciumFileDialog) for our needs. This allows me to not have to update the file dialog itself, while having relatively light code.

@generic-pers0n
Copy link

Another idea would be to just remove Audacium's modified FileDialog, and use wxFileDialog and a simple wrapper (like AudaciumFileDialog) for our needs. This allows me to not have to update the file dialog itself, while having relatively light code.

That's what I was thinking alternatively. In fact, I think a lot of things in Audacium might be modified for no apparent reason (e.g. FileDialog).

Actually, this sounds like the better solution. I just say go with it, especially since we should be using wxWidgets 3.1.3. In fact, I say a lot of things might be better off using wxWidgets' stock counterparts, but that would otherwise require more discussion.

I'll be experimenting with this for Audacium and (because why not) Saucedacity (my own derived creation ;). I'll make a pull request (if you want) related to this once I can confirm everything's good.

@generic-pers0n
Copy link

generic-pers0n commented Dec 16, 2021 •
edited

On second though, it appears that FileDialog is...more intertwined than I thought, but I just started looking at it. (At this point, it now makes sense, but that's because my mind scrambles things a lot).

For getting this solution done, it seems that the 1st is faster, but I agree more with the 2nd: it's better to drop FileDialog in the long run (in my opinion) because, like you said, it allows for lighter code. I'll still be investigating this.

Edit: The Architecture of Open Source Applications Volume I, Chapter 2 (https://aosabook.org/en/audacity.html) describes it best: "The GUI and application-specific code is all mixed together, not separated cleanly." (Now, I'd imagine things might have improved a bit, and this chapter could be outdated, but I hope this resource provides valuable information about Audacity's internals).

@AnErrupTion
Copy link
Contributor

It's harder than expected. Turns out it does modify quite a bit of stuff, and wxFileDialog doesn't have a default constructor, which is a bit problematic in that case. We'll need to revisit this.

@generic-pers0n
Copy link

It's harder than expected. Turns out it does modify quite a bit of stuff, and wxFileDialog doesn't have a default constructor, which is a bit problematic in that case. We'll need to revisit this.

Yep. Having worked on this, the default constructor appears to be the biggest issue here because compiling fails on Windows. I believe I might have figured out all (or at least some) of the places where FileDialog is used (i.e. I have a path ready), and since then they have been replaced with wxFileDialog, but I still have more testing to do.

The only thing I can think of right now is possibly deriving wxFileDialog just to merely add a default constructor. For example:

class FileDialog: public wxFileDialog
{
  public:
    FileDialog();
};

There might be other (better) options too, some of which might be better

@AnErrupTion
Copy link
Contributor

I tried that, however there are other errors (like missing functions and constructors that are in wxFileDialog, weird)

@generic-pers0n
Copy link

I tried that, however there are other errors (like missing functions and constructors that are in wxFileDialog, weird)

Ummm...well, time to rethink this through, then. I'll have to take a look at this.

I think I now see why we have a modified wxFileDialog (i.e. FileDialog) in the first place: this is why. (And there might be other reasons too, but this is one of them)

@generic-pers0n generic-pers0n mentioned this issue Dec 29, 2021
Open
@AnErrupTion
Copy link
Contributor

@generic-pers0n @jt15s Good news, it will be fixed soon, since I've found a solution. Using wxFileDialog is fine, no need for a wrapper, it just uses the constructor of wxFileDialog which takes the parent window, then instead of setting all parameters inside the constructor, we set them outside (with functions from wxFileDialog). I tested for opening a project and it seems to work.

@jt15s
Copy link
Author

jt15s commented Jan 14, 2022

Wow, that's great news! Thanks so much for looking into this!

@AnErrupTion
Copy link
Contributor

So, there is a little problem, and that seems to be with the Export file dialog. We'd have to create a separate Export dialog with the options, then use the wxFileDialog there. I'm going to first commit the other parts with the newer file dialog soon, then progressively work on the Export dialog.

@AnErrupTion
Copy link
Contributor

@jt15s I've gone ahead and pushed the commits, you can check the changes if you want to!

@generic-pers0n
Copy link

Awesome to hear this! Glad that there's a solution without using a wrapper.

What I now question is why was there a wrapper in the first place? It didn't seem necessary in the first place. Either way, at least we can (mostly) drop this wrapper, although, like was said, there still needs to be some things that have to be done with FFmpeg. For now, let's rejoice! :D

@AnErrupTion
Copy link
Contributor

although, like was said, there still needs to be some things that have to be done with FFmpeg. For now, let's rejoice! :D

What do you mean by that?

@king-dahmanus
Copy link

king-dahmanus commented Jan 15, 2022 via email

@AnErrupTion
Copy link
Contributor

@king-dahmanus Do you mean the new Export dialog or the newer file picker?

@king-dahmanus
Copy link

king-dahmanus commented Jan 15, 2022 via email

@AnErrupTion
Copy link
Contributor

@king-dahmanus Okay, one last question. Are there any accessibility issues on the Export Multiple dialog? (File ->Export -> Export Multiple...)

@king-dahmanus
Copy link

king-dahmanus commented Jan 15, 2022 via email

@AnErrupTion
Copy link
Contributor

Hm, that is weird, as the new Export dialog is essentially using the same code as the Export Multiple dialog (at least, for creation). Also, I'm trying to use the newer Windows file picker (see the original comment all above), and it's done (the old one is now gone). It's weird to me that this one has accessibility issues. I would appreciate it if you could please tell me what kind of accessibility issues those windows have. @king-dahmanus

@king-dahmanus
Copy link

king-dahmanus commented Jan 16, 2022 via email

@AnErrupTion
Copy link
Contributor

@king-dahmanus Is there a way you could post a quick screenshot of what's happening please? I can't seem to understand what's going on.

@king-dahmanus
Copy link

king-dahmanus commented Jan 16, 2022 via email

@AnErrupTion
Copy link
Contributor

@king-dahmanus If you're on Windows, go to the start menu or the search bar and type "Snipping Tool". Then, select the mode "Rectangle mode" if it's not that already, then click on "New" and select the area where you want to take the screenshot.

After that, click on the window of the Snipping Tool (to focus it) and press Ctrl + C. Finally, go to GitHub, and in your new comment press Ctrl + V to paste the image.

Hope that helps!

@king-dahmanus
Copy link

king-dahmanus commented Jan 16, 2022 via email

@AnErrupTion
Copy link
Contributor

@king-dahmanus Alright, if you say so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@AnErrupTion @generic-pers0n @jt15s @king-dahmanus