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

Change Artists list behavior #571

Closed
wants to merge 1 commit into from

Conversation

darkguinito
Copy link

Modifying artist request to supply complete artist list
Modifying artist viewer to use artist's uri instead of albumartist's uri
Changes to be committed:
modified: src/js/services/google/middleware.js
modified: src/js/services/mopidy/middleware.js
modified: src/js/store/index.js

Modify artist request to supply real artist list
Modify artist viewer to request artist from artist uri
Changes to be committed:
	modified:   src/js/services/google/middleware.js
	modified:   src/js/services/mopidy/middleware.js
	modified:   src/js/store/index.js
@jaedb
Copy link
Owner

jaedb commented Jul 31, 2020

Can you please provide background to this change? What is the need for this change, especially when the value can be changed in the UI, per user?
What are the consequences for how other people use Iris and manage their music libraries?
Typically a PR would be related to a feature request issue explaining these things.

@darkguinito
Copy link
Author

Hello James,

You right, it miss background a bit.

I use mopidy/Iris only with local files and i simply wanted to see the full list of artists.
After performing a local scan, i discover that more or less half of the artists scanned was missing.
I checked in the SQL-lite db and it seem than all artists are there.
I guess that the only thing missing was some ID3 tags on mp3 files.

As the db seem to be contain the full list of artist, i expected that iris do it as well.

I didn't know that this setting could be change from UI. I guess, you are talking about the artist library URI that can be set in the settings page ?

I try to changed it, but i was not able to figure out how to make it work.
Can you tell me where i'm wrong, i just change value to local:directory?type=artist&role=artist

But i'm a bit confused because it seem to me, that those values are hardcoded, can you correct me were i'm wrong ?
I think this could also be link to this issue from mopidy-local repository: mopidy/mopidy-local#18

If it is possible to change this setting from the UI, you can close and forget the PR and i apologize for my mistake

@jaedb
Copy link
Owner

jaedb commented Aug 1, 2020

I think you might find that you've stumbled upon a bug. When changing the setting in the UI (Settings > Advanced > Library Artists URI) and refreshing your browser, is your change retained?

@darkguinito
Copy link
Author

You're right, apparently changes does not seem retained.

I will try to understand why and I will keep you inform.

jojo141185 added a commit to seppi91/Iris that referenced this pull request Aug 9, 2020
* Display general Spotify error messages that are provided by API

* Calculating group volume on render rather than on load, fixes jaedb#534

* Removing group when last client is removed from it

* Properly cancellable processes; Playlists that refuse to load tracks

* Removing data_dir in favor of Extension.get_data_dir, jaedb#547

* Creating SearchResults component

* Search results grid layout; Tracklist

* Clearable search results

* Ditching thumbnail glow on mobile - performance suffering too much

* Play/queue actions on playlist save tracks to index for faster reuse

* Fixing context menu trigger silencing; Search form polish

* Letter spacing

* Fixing theme tweaks; Fixing prefers-color-scheme of false, resolves jaedb#549

* Buildout

* Black and flake fixups (still no check_manifest working reliably)

* Removing old helpers

* Fill prop for thumbnails on Queue and Artist for large screens

* Getting a feel for desired multi-language approach jaedb#424

* Language as window.language for access in pure function

* Exposing components for translation mapping; Queue and AAlbum views now moved to translation file

* Renaming Content to I18n to avoid naming clashes; Artist and Debug views mapped

* Mapping Playlist view

* Clickable flags; Adding links to version release notes

* Merging into main dev stream

* Mapping all remaining views - just modals and components to go

* All views done this time; Language file mapping for modals.. Will need to do destructuring in second sweep

* Halfway through modals mapping

* Final components done, dynamic language selector

* Adding error boundary around Notifications to catch issues like jaedb#565

* Responsive for < 350px devices, fixes jaedb#532

* Prod buildout

* Correcting snapcast config and path

* Updating with corrected Snapserver config file path

* Mobile polish; Performance experimentation; Loader screen; ; Minor code bugs

* Previewing items to add, in prep for random selection (jaedb#546)

* Fixing library URI setting (destructuring), possibly related to jaedb#571

* Adding generic loadItem (refactor elsewhere); Adding foundation for Add Random

* Modal detection through less-than-ideal jQuery

* Add to queue random functional

* Adding jest coverage

* Added sv.yaml

This is the first version of a Swedish translation for Iris.

* Upgrading jest

* Add German translations

* Upgrading to Babel 7; Fixing failing tests

* Disable lifecycle methods

* Buildout

* 3.51 buildout

* Adding jest to ci jobs

* Adding Code Climate coverage reporter

Co-authored-by: James Barnsley <[email protected]>
Co-authored-by: el97 <[email protected]>
Co-authored-by: Fabian Dennler <[email protected]>
@jaedb
Copy link
Owner

jaedb commented Aug 15, 2020

Thanks for your effort to create this code contribution. It has actually highlighted an oversight to consider the custom Library URI settings in this string replacement, so it'll require a bit more work at my end.

@jaedb jaedb closed this Aug 15, 2020
jojo141185 added a commit to seppi91/Iris that referenced this pull request Aug 22, 2020
* Display general Spotify error messages that are provided by API

* Calculating group volume on render rather than on load, fixes jaedb#534

* Removing group when last client is removed from it

* Properly cancellable processes; Playlists that refuse to load tracks

* Removing data_dir in favor of Extension.get_data_dir, jaedb#547

* Creating SearchResults component

* Search results grid layout; Tracklist

* Clearable search results

* Ditching thumbnail glow on mobile - performance suffering too much

* Play/queue actions on playlist save tracks to index for faster reuse

* Fixing context menu trigger silencing; Search form polish

* Letter spacing

* Fixing theme tweaks; Fixing prefers-color-scheme of false, resolves jaedb#549

* Buildout

* Black and flake fixups (still no check_manifest working reliably)

* Removing old helpers

* Fill prop for thumbnails on Queue and Artist for large screens

* Getting a feel for desired multi-language approach jaedb#424

* Language as window.language for access in pure function

* Exposing components for translation mapping; Queue and AAlbum views now moved to translation file

* Renaming Content to I18n to avoid naming clashes; Artist and Debug views mapped

* Mapping Playlist view

* Clickable flags; Adding links to version release notes

* Merging into main dev stream

* Mapping all remaining views - just modals and components to go

* All views done this time; Language file mapping for modals.. Will need to do destructuring in second sweep

* Halfway through modals mapping

* Final components done, dynamic language selector

* Adding error boundary around Notifications to catch issues like jaedb#565

* Responsive for < 350px devices, fixes jaedb#532

* Prod buildout

* Correcting snapcast config and path

* Updating with corrected Snapserver config file path

* Mobile polish; Performance experimentation; Loader screen; ; Minor code bugs

* Previewing items to add, in prep for random selection (jaedb#546)

* Fixing library URI setting (destructuring), possibly related to jaedb#571

* Adding generic loadItem (refactor elsewhere); Adding foundation for Add Random

* Modal detection through less-than-ideal jQuery

* Add to queue random functional

* Adding jest coverage

* Added sv.yaml

This is the first version of a Swedish translation for Iris.

* Upgrading jest

* Add German translations

* Upgrading to Babel 7; Fixing failing tests

* Disable lifecycle methods

* Buildout

* 3.51 buildout

* Adding jest to ci jobs

* Adding Code Climate coverage reporter

Co-authored-by: James Barnsley <[email protected]>
Co-authored-by: el97 <[email protected]>
Co-authored-by: Fabian Dennler <[email protected]>
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