✨ Add link/unlink button for the subtitle in the chart admin

[danyx23]

With descriptionShort being used as a fallback for the subtitle in a chart we now have seen some unfortunate cases that made it necessary to be able to control if the fallback should be used or not.

The reason why this is necessary is that some old charts do not have a subtitle. If we now update the indicator to have a descriptionShort then all of a sudden this desriptionShort would be used as subtitle which is maybe not what we want.

To fix this, the PR adds a link button to the subtitle text field in the chart admin. If a custom text is set then the button shows as "unlinked". If you click it the button links and you get whatever default the fallback chain provides (i.e. descriptionShort or no subtitle).

For cases where there is currently no subtitle and you want to makes sure it stays this way, set the subtitle to linked but the subtitle text to empty.

An easy way to test this is by looking at this chart that uses the Dummy indicator (which has descriptionShort).

@pabloarosado can you test this and see if it behaves the way you would expect?

[pabloarosado]

Hi @danyx23 thanks for doing this, I think it works as expected. I've tested its functionality with dummy_full instead of dummy. After merging, if you have an existing chart with no subtitle and you add a description_short, to keep the subtitle empty you simply have to go to the admin and delete the subtitle. (And for data pages, simply add in the YAML file presentation.grapher_config.subtitle: ""). So this is a good solution (much better than adding _ _ in the subtitle).

Also, as a side note, I've realised that the ETL presentation.grapher_config.subtitle is actually taken into account when creating new charts (for example, you can see that the subtitle of the indicator of dummy_full shows [grapher-config.subtitle > indicator.description-short]). Once you create a draft chart, the presentation.grapher_config.subtitle is totally ignored for grapher charts (which we knew already). So, once the draft chart is created, if you unlink and then relink the subtitle, you go from [grapher-config.subtitle > indicator.description-short] to [indicator.description-short]. This is better explained with a video:
https://github.com/owid/owid-grapher/assets/12246978/53aab554-07a9-497b-941e-d10da66fc80c
This was a bit unexpected, but it makes sense once you realise that the presentation.grapher_config.subtitle is, for grapher charts, only considered when creating a chart, not once the chart exists.

So overall I think this PR does the right thing, and it fixes the issue of not having empty subtitles, thanks!

[pabloarosado]

I can't follow the details of the code, but the functionality is the expected one.

[sophiamersmann]

Looks good to me! I played around with it a bit and the functionality feels natural to me.

A more general note: If we start linking more and more fields to metadata, I'm wondering if it would be valuable to authors if we mentioned where the default values are coming from?

[sophiamersmann]

Any particular reason you added the custom-control class? I'm asking because it adds a little too much padding on the left and puts the text area in the foreground such that the button's left border is hidden.

[danyx23]

Good point, this might well be unnecessary. I was looking through the bootstrap docs and they didn't have an example of a button with a textarea and the example use that I found had the custom-control class which is probably redundant. Will see if we can remove this.

[sophiamersmann]

I don't think I fully understand how BindAutoStringExt is different from BindAutoString. Shouldn't something like

<BindAutoString
	field="subtitle"
	store={grapher}
	auto={grapher.currentSubtitle}
/>

do exactly what we need (assuming BindAutoString supports text areas)?

[danyx23]

I tried this first but it was annoying to align the uses that were a bit different in different cases. Also, the BindAutoString class is typed with a bunch of any's because the property access can't be typed precisely enough. The approach with getter and setter functions is trivial to type precisely so I prefer that solution.

Do you think I should rewrite the other uses and remove BindAutoString?

[sophiamersmann]

Nah, no need to rewrite. I was just wondering if I was missing something essential here :)

[danyx23]

@pabloarosado thanks for testing it! Interesting find about the indicator grapher config subtitle being used initially but not anymore later on. I think this is ok-ish for now but let's keep looking out for weird cases like this. In general I think our appetite to improving this is high, it's just a question of how early we want to chip away further vs. plan things a bit more coherently for next year.

@sophiamersmann thanks a lot for your review! I agree, making it clearer where the defaults are coming from will be very useful. When we start using defaults for all properties in the charts admin it might be quite nice to be able to switch between 3 different views: the merged chart configs, only the ETL one and only the "chart" one. Definitely something to think about more :)