search

segler-alex / RadioDroid

radio browser app that uses www.radio-browser.info on android
GNU General Public License v3.0
755 stars 156 forks source link

Remove Extra MPD Icon #653

Closed zo-shin closed 4 years ago

zo-shin commented 4 years ago

The MPD icon at the top right of settings should be removed because there is music player daemon option there.

segler-alex commented 4 years ago

does anybody now why it has been added there?

werman commented 4 years ago

It's there because I didn't specifically remove it when settings fragment is visible.

And the this button itself triggers new MPD dialog.

segler-alex commented 4 years ago

should we:

  1. remove it?
  2. give it an icon?
  3. let it as is?
werman commented 4 years ago

I don't think there is an icon which will clearly tell user that it is MPD.\ It's an mpd preference in settings which could be removed because it does the same as MPD button.

zo-shin commented 4 years ago

I think the icon should be removed. 1 - It's a little bit weird to single out one option out of others. 2 - Users can fully understand the usage of MPD by the current option, so the extra icon is not very necessary.

werman commented 4 years ago

@segler-alex The icon should be there, you removed it for all screens.

segler-alex commented 4 years ago

sorry, then i did not understand what the consensus was. please explain it.

segler-alex commented 4 years ago

oh, there was no consensus...

segler-alex commented 4 years ago

one said, remove icon, the other said, remove setting

werman commented 4 years ago

Without the button the only way to summon the mpd dialog is via settings which is several clicks away however it is meant to be quickly accessible (user shouldn't go to settings to pause playback in mpd or change volume on server).

However I could agree that this button dangling for every user isn't great. I think the best solution will be displaying it only if there is at least one mpd server added by user.

segler-alex commented 4 years ago

good, i have to check how to do this.

werman commented 4 years ago

@segler-alex Better put Closes: into body of the commit for reverts not to cause issue to close. And make commit header meaningful.

segler-alex commented 4 years ago

ok, it is fixed with the next commit

segler-alex commented 4 years ago

i just clicked revert commit in my git frontend :)

werman commented 4 years ago

I meant when you are creating commit place "Closes:" into body.

And instead of creating new MPDServersRepository in onCreateOptionsMenu it's better to get MPDClient from the app and ask it if there are servers.

@segler-alex Could you create pull requests instead of committing directly into master? This would allow me to give feedback if I see something wrong.