[LiquidDoc] Improve hover behaviour for multi-line descriptions and examples #850
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
jamesmengo
changed the title
jamesmengo
force-pushed
the
jm/indentation_hover
branch
2 times, most recently
from
663ce62 to
81a3bef
Compare
| return line.slice(minIndent); | ||
| }), | ||
| ].join('\n'); | ||
| } |
There was a problem hiding this comment.
There is a similar utility that exists in the prettier plugin but I chose to implement this here instead.
- The only shared dependancy is
liquid-html-parser - I explored moving it into a shared utility into
liquid-html-parserand didn't feel that the benefit was > the cost of duplication here. - I highly doubt this will change, and if it does we don't want to change the prettier behaviour
| @@ -51,7 +51,7 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider { | |||
|
|
|||
| if (liquidDoc.description) { | |||
| const description = liquidDoc.description.content; | |||
| parts.push('', '**Description:**', '\n', `\`\`\`plaintext\n${description}\n\`\`\``); | |||
| parts.push('', '**Description:**', '\n', description); | |||
jamesmengo
force-pushed
the
jm/indentation_hover
branch
from
81a3bef to
278a457
Compare
| function fixIndentation(text: string): string { | ||
| const lines = text.split('\n'); | ||
|
|
||
| if (lines.length <= 1) return text; | ||
|
|
||
| const nonEmptyLines = lines.slice(1).filter((line) => line.trim().length > 0); | ||
| const indentLengths = nonEmptyLines.map((line) => { | ||
| const match = line.match(/^\s*/); | ||
| return match ? match[0].length : 0; | ||
| }); | ||
|
|
||
| if (indentLengths.length === 0) return text; | ||
|
|
||
| const minIndent = Math.min(...indentLengths); | ||
|
|
||
| return [ | ||
| lines[0], | ||
| ...lines.slice(1).map((line) => { | ||
| if (line.trim().length === 0) return line; // Skip empty lines | ||
| return line.slice(minIndent); | ||
| }), | ||
| ].join('\n'); | ||
| } |
There was a problem hiding this comment.
Are we missing some logic here? I can't properly render multiline description.
There was a problem hiding this comment.
Great question! This is intentional. Example 2 in the PR description illustrates an example of how I came to the realization that our intention to allow for easy indenting in the description was creating more problems than value.
-
This required us to render as
plaintext, which resulted in the issue where subsequent lines were unexpectedly indented. -
When content overflows outside of the tooltip width, a new line is automatically added. This, to me, breaks the paradigm of allowing users to format their text as desired. If we provide users with the ability to manage new lines and indenting, new lines should only be added
IFFthe user manually inputs them.
Rationale
I think the large majority of users will benefit more from multi-line values not having a weird indent + rendering with a consistent font.
The previous approach also felt 'broken' when the hover tooltip was adding its own newlines when content was overflowing.
With this approach, it's still possible for users to add newlines / indentation if they want to, though this now feels hacky.
I'm ok with this tradeoff, as I no longer think that control over indentation / whitespace for descriptions is a first-class feature.
There was a problem hiding this comment.
I think the large majority of users will benefit more from multi-line values not having a weird indent + rendering with a consistent font.
it's still possible for users to add newlines / indentation if they want to, though this now feels hacky.
I think we could revisit this if requested by someone. But looks like a fair tradeoff for now.
---- - Render Description as markdown instead of plaintext - Fix indentation for multi-line examples and descriptions
jamesmengo
force-pushed
the
jm/indentation_hover
branch
from
278a457 to
d42ed61
Compare
What are you adding in this PR?
Closes https://github.com/Shopify/developer-tools-team/issues/600
The Problem:
1: Hovering renders extra indentation for multi-line examples and descriptions
2: Rendering descriptions as plaintext results in formatting inconsistencies
This PR:
descriptionandexamplecontentdescriptionto be rendered asmarkdownon hover. This prevents the formatting inconsistency while still allowing users to render newlines if they desire, though it would be manual.What did you learn?
Before you deploy
changeset