musicbrainz-server icon indicating copy to clipboard operation
musicbrainz-server copied to clipboard

MBS-7646: Show localized artist aliases in more places

Open derat opened this issue 11 months ago • 1 comments

MBS-7646

Try to find artists' primary aliases in the user's locale and display them alongside disambiguation comments in search results and artist pages.

This is currently just a proof-of-concept.


I'm tired of reading arguments like the one at https://musicbrainz.org/edit/109219215, so I'm uploading this to get some feedback about whether this seems like a reasonable approach to pursue. Along with the TODOs in the code, I suspect that this code doesn't yet work for indexed searches -- I've been testing it with direct searches in my local dev environment. I'm also not sure if aliases should be displayed in more (or fewer) places, and whether there's any way to generalize this to support additional entity types instead of only artists.

derat avatar Feb 28 '24 21:02 derat

@mwiencek @reosarevok @yvanzo Early feedback appreciated. :-)

derat avatar Feb 28 '24 21:02 derat

I would love to test and feedback, but I think that would involve configuring the search server on my side? It was a bit of a rigmarole to get the test server running on Win at all so I'm a bit hesitant.

I would love to see a screenshot - this sounds really useful! I am keen to see how it's displayed.

Aerozol avatar Mar 16 '24 09:03 Aerozol

@Aerozol no need for a search server; the localized names are shown in the results when you do a direct search (e.g. http://localhost:5000/search?query=tchaikovsky&type=artist&limit=25&method=direct).

There also isn't too much to see, though; the disambiguation is just rendered as "Pyotr Ilyich Tchaikovsky, Russian romantic composer" instead of "Russian romantic composer". :-)

derat avatar Mar 16 '24 11:03 derat

Sorry for the delay in comment, I finally (with further help from monkey) managed to wade through all my Win OS/Ubuntu VM issues, and 'test' this. Hopefully nobody was waiting for me!

This is a easy 'yes' from me. MB isn't mobile friendly anyway, so I don't see a downside in having this take up some of the empty horizontal space. And it is indeed a constant cause of friction re. disambiguation edits.

My only thought is that we may want to distinguish between the automatically displayed content by putting it in italics, in case users get confused, but it would also be uglier.

The TODO comments are too technical for me to address.

My only wish is that this was extended to more (all) entity types/searches. And some day we maybe automate even more of the disambiguation, like RYM does - e.g. 'rock group from Bulgaria' can all be automated. cough bumps into derat cough

Great stuff!

Aerozol avatar Mar 26 '24 03:03 Aerozol

Thanks for taking a look!

I'll try to spend some more time soon working on the remaining issues, e.g. getting this to work for indexed search. It'd definitely be nice to support other entity types too, but I'll need to look into how hard it'll be to pass the aliases through there. And your suggestion to italicize the alias also seems like it'll help reduce confusion -- I'll try to see how that looks.

One other consideration is that there are probably a bunch of entities that already have Latin names in their disambiguations where this would result in near-duplicated text. IIRC I was thinking of avoiding displaying the alias if it's already present in the disambiguation or if it matches the entity name, but there are likely still many cases that would be missed due to alternate spellings, diacritics, etc.

derat avatar Mar 26 '24 11:03 derat

Ninja'd! I was just drafting another comment that we might run into some situations where there is some very repetitive text. Because of text in the disambiguation, but also where the primary alias is very similar to the artist name (I don't know if this avoids adding it if it's 100% the same?)

Aerozol avatar Mar 26 '24 12:03 Aerozol

Avoiding adding it if the same as artist name makes a lot of sense to me. I would not hide it if it's already in the disambiguation - it should not be there anyway, so this will make it more obvious so people can remove it :)

reosarevok avatar Mar 26 '24 13:03 reosarevok

After some musicbrainz-docker help from @yvanzo in MBVM-95, I've started looking at this again. Here's a screenshot of what it looks like after italicizing the localized aliases:

image

derat avatar Apr 06 '24 14:04 derat

I like the italics :) (I should have just tested it in the browser console before suggesting it, sorry!)

Another step towards clarity could be mouse-over text for the italics, something like: "This artists' primary alias in your [browser/account(?)] locale", but I don't think it's urgent.

Aerozol avatar Apr 07 '24 06:04 Aerozol

Another step towards clarity could be mouse-over text for the italics, something like: "This artists' primary alias in your [browser/account(?)] locale", but I don't think it's urgent.

Thanks! I've added "Primary alias in your language" title text.

derat avatar Apr 11 '24 00:04 derat

I added some simple Selenium tests, although I assume they'll need some work (still no way to run them locally with musicbrainz-docker, right?).

derat avatar Apr 11 '24 14:04 derat

The new tests pass:

MBS-7646: Show localized primary aliases alongside disambiguations                                                                                               
[17:16:41.997Z] sendKeys target="css=#headerid-query" value="tchaikovsky${KEY_ENTER}"  
[17:16:43.086Z] waitUntilUrlIs target="/search?query=tchaikovsky&type=artist&method=indexed" value=""  
[17:16:43.098Z] assertText target="css=#content table.tbl td a" value="Пётр Ильич Чайковский"  
  ✓ should be equal  
[17:16:43.131Z] assertText target="css=#content table.tbl td .comment" value="(Pyotr Ilyich Tchaikovsky, Russian romantic composer)"  
  ✓ should be equal  
[17:16:43.160Z] click target="css=input[name=\"method\"][value=\"direct\"]" value=""  
[17:16:43.223Z] click target="css=div.searchform button[type=\"submit\"]" value=""  
[17:16:44.225Z] waitUntilUrlIs target="/search?query=tchaikovsky&type=artist&limit=25&method=direct" value=""  
[17:16:44.238Z] assertText target="css=#content table.tbl td a" value="Пётр Ильич Чайковский"  
  ✓ should be equal  
[17:16:44.265Z] assertText target="css=#content table.tbl td .comment" value="(Pyotr Ilyich Tchaikovsky, Russian romantic composer)"  
  ✓ should be equal  
[17:16:44.291Z] click target="css=#content table.tbl td a" value=""  
[17:16:45.745Z] waitUntilUrlIs target="/artist/9ddd7abc-9e1b-471d-8031-583bc6bc8be9" value=""  
[17:16:45.758Z] assertText target="css=.artistheader a" value="Пётр Ильич Чайковский"  
  ✓ should be equal  
[17:16:45.791Z] assertText target="css=.artistheader .comment" value="(Pyotr Ilyich Tchaikovsky, Russian romantic composer)"  
  ✓ should be equal

derat avatar Apr 11 '24 17:04 derat

@brainzbot, retest this please

derat avatar Apr 12 '24 14:04 derat

@reosarevok and @yvanzo, I think this is ready for review now (when you have time). :-)

derat avatar Apr 15 '24 22:04 derat

@mwiencek, would you mind taking a look at this one too? Thanks!

@brainzbot, retest this please

derat avatar Apr 22 '24 23:04 derat

@mwiencek, mind taking another look?

derat avatar Apr 25 '24 13:04 derat

Thanks for all the help getting it merged!

derat avatar Apr 26 '24 17:04 derat