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

FileDialog: Add URLs only if they're not present #7430

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

khoidauminh
Copy link
Contributor

@khoidauminh khoidauminh commented Aug 5, 2024

As #7405 states, all user-saved bookmarks/URLs on the sidebar is lost when LMMS launches the FileDialog, which affects other QT apps that share the same file dialog in the system.

This does:

  1. Keep the original code, but load all the preexisting URLs and only insert to the sidebar the URLs that aren't there. Some users might have their own configuration and want it to stay that way, therefore we should keep all the original URLs.
    2. Store a list of the old URLs and restore them when the destructor of FileDialog is called. Removed, see reason

I initially wanted to have FileDialog(...) open the sidebar as is, but that would mean removing the implementation the previous contributor intended.

include/FileDialog.h Outdated Show resolved Hide resolved
src/gui/modals/FileDialog.cpp Outdated Show resolved Hide resolved
@khoidauminh
Copy link
Contributor Author

khoidauminh commented Aug 11, 2024

I realized that the user may also want to add bookmarks inside LMMS. This PR would've made it impossible to do that because the the sidebar will always revert to the old state when it exists. The steps needed to scan for new bookmarks are too complicated and unnecessary for now, so I'm going to remove the 2nd task of this PR.

@khoidauminh khoidauminh changed the title FileDialog: Add URLs when needed & restore old sidebar state on exit FileDialog: Add URLs only if they're not present Aug 11, 2024
@sakertooth
Copy link
Contributor

I realized that the user may also want to add bookmarks inside LMMS. This PR would've made it impossible to do that because the the sidebar will always revert to the old state when it exists. The steps needed to scan for new bookmarks are too complicated and unnecessary for now, so I'm going to remove the 2nd task of this PR.

Would switching to using just the QFileDIalog work? I believe it handles all of this for us and more automatically.

@khoidauminh
Copy link
Contributor Author

khoidauminh commented Aug 11, 2024

@sakertooth It could be possible. However LMMS's FileDialog currently has some custom methods. I can try to migrate to QFileDialog but this is a bit out of the scope of this PR at the moment

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.

FileDialog::FileDialog(...) resets (and overwrites) the Sidebar URLs in LMMS and other QT apps
4 participants