elementary / music

Music player and library designed for elementary OS
https://elementary.io
GNU General Public License v3.0
145 stars 49 forks source link

Less intrusive import error handling #604

Closed jeremypw closed 2 years ago

jeremypw commented 3 years ago

Fixes #470

Screenshot from 2020-09-28 12 29 20@2x

Ideally there should be an option to install missing plugin(s) but that will be left for a future PR.

The styling is pretty basic but can be improved if this is accepted.

danirabbit commented 3 years ago

Hey thanks for working on this and trying to provide better error feedback here! I agree that offering to trash these files is probably not great and showing them in Files is probably more productive.

I'm not sure a toast is a good option for this since toasts automatically expire and an import may be a long-running operation where the user steps away from the computer. It's also kind of weird for a Toast to launch a dialog, imo. Toasts should probably be used for super quick feedback and convenient one-click actions. A dialog is probably still appropriate for errors, especially if there is an option to always ignore errors

There should probably be some secondary text that explains the implications of an import error. For example that these files won't appear or be playable in Music.

I'm not sure always showing the library is the most helpful here, especially if the import is being performed from a different folder. This button should probably say something like "Show Files" and open the import location. It should probably be in the primary action position in the dialog and not floating in the center of the dialog.

"Ignore" would probably be a better dismiss action since it makes it explicit that you're electing to take no action on this problem and not just that you're closing the window.

Regarding the checkbox, I wonder if we really need this feature. It sounds like the original issue is that the import error dialog is shown while performing an automatic import. We probably don't want this dialog to not appear when performing a manual import. Failing silently is probably not expected for explicit action. But in the case of an automatic library rescan, failing silently is probably not a big deal. So it might solve the problem to fail silently on automatic imports without adding this new setting

jeremypw commented 3 years ago

Thanks for a lot of useful points - I'll get to work on them. With regard to replacing the toast, do you suggest launching the dialog directly or perhaps putting some kind of error warning icon into the headerbar or somewhere which launches the dialog when pressed (and then disappears)?

Where a file is reported as busy (probably very rare in practice) I am not sure how to find out why - unless it is in the original error message. I used my own (draft) error summaries instead of outputting each error message as received. Is it worth providing access to the full error details?

danirabbit commented 2 years ago

Closing since this targets old Music