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

Feature select player #251

Merged
merged 2 commits into from
Mar 30, 2019
Merged

Feature select player #251

merged 2 commits into from
Mar 30, 2019

Conversation

DanielVoogsgerd
Copy link
Contributor

And here is the feature itself. I verified that it work in my usecases. But yours might be different. If you don't mind testing it first that would be awesome.

This branch is based on my branch from my other PRs (#249, #250) as I needed command line arguments to implement this.

@Alexays
Copy link
Owner

Alexays commented Mar 28, 2019

Nice!
Do we also want to change the default config with --player spotify?

@DanielVoogsgerd
Copy link
Contributor Author

DanielVoogsgerd commented Mar 28, 2019

I'd prefer not too. Right now, if the --player is left empty it will update the widget depending on the last player that had an event. Considering nothing is provided I think that is ideal behavior. I would suggest maybe adding --player spotify to the documentation, to showcase the functionality.

Now I'm thinking of it, it might be a good idea to change it in the example config as well, but I'll let you decide on that matter.

@Alexays
Copy link
Owner

Alexays commented Mar 28, 2019

For me, it'll better to specify --player spotify since it's a spotify module in the default conf

@DanielVoogsgerd
Copy link
Contributor Author

DanielVoogsgerd commented Mar 28, 2019

How do you suggest implementing dynamic player recognition in that case then? I'm fine with a default but do like the possible dynamic behaviour. On top of that changing the default to --player spotify is not backwards compatible, which current behaviour is.

May I suggest we add the --player spotify to the default config. To me, it makes more sense that way.

@Alexays
Copy link
Owner

Alexays commented Mar 29, 2019

A custom module can return a json with an icon and/or a class specified, so we can specify an icon for each supported players and also a class with a background color, what do you think?

@DanielVoogsgerd
Copy link
Contributor Author

Yeah sure, I suspect that that is more in the backend compartment.
Do you mind finishing off that part?

@Alexays
Copy link
Owner

Alexays commented Mar 29, 2019

@DanielVoogsgerd Sure, i'll do it tomorrow ;)

@Alexays Alexays merged commit 3072582 into Alexays:master Mar 30, 2019
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.

2 participants