This removes the 'Unmount' context menu in the Places panel for discs
corresponding to / and /home.
It does not make much sense to offer an option that will always fail.
BUG: 399659
elvisangelaccio |
Dolphin | |
VDG |
This removes the 'Unmount' context menu in the Places panel for discs
corresponding to / and /home.
It does not make much sense to offer an option that will always fail.
BUG: 399659
No Linters Available |
No Unit Test Coverage |
Buildable 3606 | |
Build 3624: arc lint + arc unit |
Agreed in principle.
Yes, we generally prefer to disable unavailable options rather than removing them. The HIG says:
If some of the program’s settings are only applicable in certain contexts, do not hide the inapplicable ones. Instead, disable them and hint to the user why they’re disabled. Exception: it is acceptable to hide settings for non-existent hardware. For example, it’s okay to hide the touchpad configuration when no touchpad is present.
This doesn't seem like it falls under the exception, so let's disable it instead of not showing it at all. Bonus points if the disabled item's tooltip tells the user why the disk can't be unmounted!
I wonder if this is the correct way to make the change, though. Perhaps providesTearDown should get a third condition that checks for "can unmount", if Solid provides that functionality. @bruns or @broulik, do you know if it does?
src/panels/places/placesitemmodel.cpp | ||
---|---|---|
215 ↗ | (On Diff #42985) | Please use QUrl::fromLocalFile(QDir::rootPath()) and QUrl::fromLocalFile("/home") instead. |
src/panels/places/placespanel.cpp | ||
---|---|---|
192 | We should not hardcode "/home" here, but derive it from the user home directory. |
We just got a bug report for this: https://bugs.kde.org/show_bug.cgi?id=399659
So please add BUG: 399659 to the summary here in Phab and then do arc amend locally to update the git commit message from the Phab diff.
@elvisangelaccio, does this look good now?