Conversation
|
@domoritz: working on this issue I noticed signal strings created by Vega-Lite all use escaped double quotes
would become
Is there a reason for not using single quotes? Maybe I'm missing some edge-cases? |
|
At least one reason would be that " are slightly easier to use with the default ' in the Vega-Lite code base. I suggested changing to " in #5361. We could do it but should merge any big pending prs first. I'm supportive of it if you would like to tackle the task (in a separate pull request of course). |
|
@domoritz I'll be offline the next two weeks, but when I'm back, I would be happy to dig into the formatting of signal strings in a future pull request (after I have finished working on the backgrounds for text marks). |
src/compile/mark/encode/tooltip.ts
Outdated
| expr: 'datum' | 'datum.datum' = 'datum', | ||
| ): VgValueRef { | ||
| // tooltip fields with a format property are no strings | ||
| const fieldDefWithFormat = channelDef as {field: string; type: string; format: string}; |
There was a problem hiding this comment.
how do you know that the type here is {field: string; type: string; format: string}?
There was a problem hiding this comment.
I'm still wrestling a bit with the elaborate type system in this repo 😅
I have replaced the cast with a type guard.
| if (fieldDefWithFormat?.type === 'nominal' && !fieldDefWithFormat.format) { | ||
| const fieldString = `datum["${fieldDefWithFormat.field}"]`; | ||
| return { | ||
| signal: `isValid(${fieldString}) ? isArray(${fieldString}) ? join(${fieldString}, '\\n') : ${fieldString} : ""+${fieldString}`, |
There was a problem hiding this comment.
Why do we need to convert invalid values to strings? Can we reuse logic here?
There was a problem hiding this comment.
This is in line with the way formatSignalRef handles nominal fields:
return {signal: `isValid(${field}) ? ${field} : ""+${field}`};
I think to render null, undefined, or NaN as strings in tooltips and area descriptions?
I'm sorry for that. I did add an action secret and tried to trigger the CI by including |
|
All good. Our setup for external forks is a bit wonky because of some restrictions GitHub actions had (have?). I often make an internal copy to get the build to work correctly. |
|
Is it true that if there is a format, the tooltip is not a string? It's a string afterwards but I guess it's not likely to be a multi line string, right? |
The comment was unclear, I have removed it. The logic is:
I hope this answers your question? |
domoritz
left a comment
There was a problem hiding this comment.
Looks good. Let's get the tests to pass and I can merge.
src/compile/mark/encode/tooltip.ts
Outdated
| ): VgValueRef { | ||
| // tooltip fields that are not nominal or have a format property are no strings | ||
| if (isFieldDef(channelDef) && 'type' in channelDef && channelDef.type === 'nominal' && !channelDef.format) { | ||
| if (isFieldDef(channelDef) && 'type' in channelDef && isDiscrete(channelDef.type) && !channelDef.format) { |
There was a problem hiding this comment.
isn't 'type' in channelDef always true?
|
I merged your changes into #9678. |
|
We should make you a maintainer. Would you like to (https://github.com/vega/.github/blob/main/project-docs/MAINTAINERS.md)? |
Internal version of #9647 --------- Co-authored-by: Bas Broekhuizen <bas.broekhuizen@gmail.com> Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
|
Thank you. Merged in #9678 |
|
@domoritz Thanks for reviewing and merging! If you have time, maybe you can have a look at the corresponding PR in vega-tooltip that is necessary to render the newlines in the tooltip. |
PR Description
This PR adds support for rendering newlines in tooltips and potentially closes #7564.
A new function has been added in the tooltip encoder that transforms tooltip values that are arrays into strings joined by newline characters. This allows multi-line tooltip content to be displayed correctly.
To support this behavior visually, a small CSS update is required in the vega-tooltip package — see the accompanying PR in that repository.
Example
With the new code, this example spec (open in editor) will render a tooltip like this: