jesus2099 / konami-command

power‐ups for various web sites
118 stars 25 forks source link

Use MBS disambiguation styling (comment) #345

Closed culinko closed 6 years ago

culinko commented 7 years ago

Notes by @jesus2099

In this original post I have underlined the text that is related to the recording display enhancement triggered by this ticket: Use MBS disambiguation styling (comment) . More tickets will eventually be created for other topics, after some discussions.

Thanks very very much @culinko for your interest in my script(s).

Original post by @culinko

Hello there :)

I've been using your mb userscripts for a really long time and I'm very grateful that you are still maintaining them. I've come across a question and two requests about this particular userscript I'd like to know. Here they are:

Let me know what do you think about these ideas and also if you'd prefer me to split these into separate issues on github, I'll gladly do it. Thanks for any feedback, much appreciated!

jesus2099 commented 7 years ago

Thanks very much for the ticket @culinko. I think the features you tell are in two scripts. I think I should probably merge them... ☺

culinko commented 7 years ago

Well, I am using both of the scripts, so you might be right about that :) However there might be people editing classical releases which have (as far as i remember) performers on recording and composers on track, so they might not like it bundled with the other script if they use the other one.

As for my first question, I still wonder, is the comment highlighting intentional?

jesus2099 commented 7 years ago

I think I remember that there is highlighting whenever track title is different from recording title plus comment, indeed. And it was intentional. Because it can show a possible mistake in some cases. Like a non remix track linked to a remixed recording.

culinko commented 6 years ago

Hello again! I decided that I don't want to use the highlighting recording comments feature anymore. I still do want the comments to be shown, but in grey color and without the shadow. Because I won't use the feature anymore, I decided to remove the comment from the comparison string as well, so the script wouldn't need to use additional resources. In mb. INLINE STUFF, I changed so far (I don't have any previous experience with scripting btw. so everything I did was an intuition):

I changed row 267 to var sameCompleteName = aRec.textContent == recnameNet[mbid].name; and row 287 to if (markTrackRecNameDiff && recnameNet[mbid].comment) {

If I am correct, this should remove the comment from the track/recording comparison and should display the comment whenever it exists. Interestingly enough, this doesn't change anything in the script's functionality so far, because if a recording has a comment, it's automatically different from the track name (so I don't know why do you need to compare it in the first place :P).

Now for the style part, I am still struggling to find out the cleanest solution possible. So far I got it working by changing the value recdis in row 289 to something like recdis2 and then inserting a new row in position 88 which is something like css.insertRule("span." + userjs + "recdis2 { color: #999}",1);. I don't know how the original row works (for example I don't know what is the "a", the "span." the square brackets and why is the userjs called twice), but this method of adding the second css.insertRule feels kinda hacky to me. So I was wondering, is there any way to keep the original value recdis in row 289 and just modify the row 87 in a way that it keeps the grey shadow + maroon color for the recname value and then I define the recdis with no shadow and grey color? Or is it impossible to define two different color schemes in one css.insertRule?

I am sorry for spamming this ticket, I would message you on Discourse but private messages are still disabled there... Many thanks for your assistance!

jesus2099 commented 6 years ago

It’s good to have this here instead of private. :) Sorry I have no time nowadays but I think my goal was that a track called A (sax version) should not be considered different than either a recording called A (sax version) or a recording called A with a sax version comment. This is why I use the comment in the comparison.

culinko commented 6 years ago

Oh, that scenario never occurred to me and is actually very clever!

Don't worry, I will try to find more info about css.insertRule or bother someone on irc about the color thingy :)

jesus2099 commented 6 years ago

I can help you for the styling. insertRule takes a string that should follow the same syntax as a cascading style sheet (.css) files. Which one of the three opening post items is your style question? What do you want to change to the styling, in a few words? I guess it’s about how is styled a track that differs from current recording, right?

culinko commented 6 years ago

I want to change the comment highlight from maroon text color and grey shadow to grey text color and no shadow. I want to keep other highlights untouched. Attaching a picture:

comment

So far I tried changing the row 87 to:

css.insertRule("a[" + userjs + "recname] { text-shadow: 1px 2px 2px #999; color: maroon}, span." + userjs + "recdis { color: #999}", 0);

or

css.insertRule("a[" + userjs + "recname { text-shadow: 1px 2px 2px #999; color: maroon}], span." + userjs + "recdis { color: #999}", 0);

However no success so far, because I don't know what is the "a" and the square brackets around recname.

jesus2099 commented 6 years ago

You can go to the 87th row, indeed:

            css.insertRule("a[" + userjs + "recname], span." + userjs + "recdis { text-shadow: 1px 2px 2px #999; color: maroon }", 0);

And replace it with:

            css.insertRule("a[" + userjs + "recname] { text-shadow: 1px 2px 2px #999; color: maroon }", 0);
            css.insertRule("span." + userjs + "recdis { color: #999 }", 0);

Maybe I should use this in the script. Please use it for a while and tell me what you think…

Could you check boxes in your initial post when you consider subjects are closed?

jesus2099 commented 6 years ago

Sorry I edited my code just above because I didn’t get quite your request yet. Could you send me the link to the release you pictured so I can check?

jesus2099 commented 6 years ago

Found it: https://musicbrainz.org/release/19950f6f-9c91-43d1-aa7b-dae2c05405b1 Looks good with your setting… Tell me but I think I could include it.

jesus2099 commented 6 years ago

I will simply use the MBS’ grey .comment style class. Don’t change your script, just leave the auto‐update fix it for you. :)

And it does work good with the case I described: track name = recording name + " (" + recording comment + ")". This track has a span.name-variation by MBS because indeed track name ≠ recording name but my script does not mark it as different and does not display the comment in double.

jesus2099 commented 6 years ago

FYI both script are already marked for merge in #129.

culinko commented 6 years ago

Yeah this was my initial thought, that I can't have two different color values in one css.insertRule. I was wondering if the index at the end should be 1 e.g. css.insertRule("span." + userjs + "recdis { color: #999 }", 1); because it's a second insertRule, but I see you dropped it for the .comment style class, which looks much cleaner. It looks exactly as I envisioned, hopefully other people won't crucify me for this change, because you changed it globally in your script so everyone who updates the script will have it this way from now on.

There is one additional thing I was wondering about today, this time regarding the tooltip. In row 270, there is: ntit = (ntit ? ntit + " —\u00a0" : "") + "track name: " + aRec.textContent + "\r\n≠rec. name: " + recnameNet[mbid].name; which looks like this:

tooltip1

Can I ask why is the ntit value compared in (ntit ? ntit + " —\u00a0" : "") and why are the additional things (the title and dashes) displayed? I removed the mentioned part from the row so only ntit = "track name: " + aRec.textContent + "\r\n≠rec. name: " + recnameNet[mbid].name; would remain, which would look like this (imo much more clear):

tooltip2

So I am wondering, am I losing any functionality with this change?

jesus2099 commented 6 years ago

Sorry I created a discussion to keep GitHub tickets focused on single/simple tasks. :)

Could you please repost over there and we’ll create one ticket for each requests that will come out of the discussion, OK? :)

I am now renaming and closing this ticket to consider this single enhancement: Use MBS disambiguation styling (comment).