-
Notifications
You must be signed in to change notification settings - Fork 16
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
Adaptive Import-dialog #102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good start so far.
I have a couple of suggestions:
- File pickers aren't transient with the parent window with the current MR, so I suggest to do so.
- File pickers should only open appropriate file types by default. If I recall correctly, NewPipe exports JSON files. So, to import NewPipe list, the file picker should filter JSON. I believe it's this one https://docs.gtk.org/gtk4/property.FileChooser.filter.html.
- File pickers don't have a title, so the title bar is entirely empty.
For NewPipe, I suggest to call it "Select NewPipe subscription file". For YouTube, I don't know exactly what we're supposed to import.
c871b31
to
922c6b7
Compare
I have applied your suggestions. FileFilters were already in use and worked for me (NewPipe: JSON, YouTube: CSV). |
Huh, sorry for that then. I was really confused by the file picker displaying "(None)" in the filter list. That said, I just tested the artifact and it looks like the file pickers still aren't transient with the parent window. |
I just checked the docs again, and I think what you are refering to as transient is transient and modal:
Update: I now updated the MR to also make things modal. |
922c6b7
to
889c87b
Compare
OH, sorry for saying the wrong term all along! 😅 I tested the build artifact and I can confirm that filepickers are now modal. |
Licensing
Description
This exchanges the dialog to be a
AdwMessageDialog
. The transient-for property is still set, but I am still unsure it applies.Linked Issue
Relates to #95.