[lexical-playground] Fix: tabs do not show strikethrough

[vantage-ola]

Description

Made some changes to .PlaygroundEditorTheme__textStrikethrough to show strikethroughs on tabs

Closes #6808

Before

After selecting the whole text, it shows the strikethrough is present

After

The slight change in css makes the strikethrough visible

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
lexical	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 22, 2024 10:53pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 22, 2024 10:53pm

[github-actions]

size-limit report 📦

Path	Size
lexical - cjs	30.94 KB (0%)
lexical - esm	30.8 KB (0%)
@lexical/rich-text - cjs	39.6 KB (0%)
@lexical/rich-text - esm	32.66 KB (0%)
@lexical/plain-text - cjs	38.2 KB (0%)
@lexical/plain-text - esm	29.92 KB (0%)
@lexical/react - cjs	41.37 KB (0%)
@lexical/react - esm	34 KB (0%)

[vantage-ola]

i took another approach to fixing this.., i just jumped into it without thinking and was in an hurry to fix it... I had even forgotten both underline and strikethrough existed...

the reason I used white-space: pre; is because normally HTML collapses multiple spaces into one, but white-space: pre treats them like <pre> tags

[vantage-ola]

i took another approach to fixing this.., i just jumped into it without thinking and was in an hurry to fix it... I had even forgotten both underline and strikethrough existed...

the reason I used white-space: pre; is because normally HTML collapses multiple spaces into one, but white-space: pre treats them like \<pre\> tags

no need for them actually. 😅 silly me.. i was using display: inline-block; while trying out things...

[etrepum]

Don't bother trying to replace the spaces, that will cause problems. I suggested a possible approach in my last comment, this workaround only needs to apply to tab nodes. If you add a class so they can be targeted you should be able to work around it for only those nodes and leave the other CSS alone so this fix doesn't break anything else.

[vantage-ola]

I tried soemthing... but it breaks something in the tests... but i think the strikethrough/underlining or both works, atleast from what i tested in the playground

Argument of type 'TextNode' is not assignable to parameter of type 'CodeHighlightNode | TabNode | LineBreakNode'.
      Property 'getClassName' is missing in type 'TextNode' but required in type 'TabNode'.

    265       let node = getFirstCodeNodeOfLine(firstSelectionNode);
                                                ~~~~~~~~~~~~~~~~~~

      packages/lexical/src/nodes/LexicalTabNode.ts:52:3
        52   getClassName(): string {
             ~~~~~~~~~~~~
        'getClassName' is declared here.

[etrepum]

Not sure why that check failed, let's see if it happens again once the other issues are fixed. I can't review the functionality because it didn't deploy.

[etrepum]

This is far more work than it needs to be, the format classes are already assigned by the super implementation. You only need to add the class for TabNode in createDOM. This class should be defined in the theme, not hard-coded in this implementation.

[vantage-ola]

ok, thank you. will get to work on this. and make necessary adjustments.

[vantage-ola]

so, only createDOM is needed in LexicalTabNode, but I noticed a number of tests are failing due to a new class name being introduced to lexical. And while testing, I dont think the pseudo element approach will ever work(but i may be wrong though)

for example:

 ● LexicalSelection tests › Paste text, move selection and delete word forward (#138)

    expect(received).toBe(expected) // Object.is equality

    Expected: "<div contenteditable=\"true\" style=\"user-select: text; white-space: pre-wrap; word-break: break-word;\" data-lexical-editor=\"true\"><p class=\"editor-paragraph\" dir=\"ltr\"><span data-lexical-text=\"true\">ABD</span><span data-lexical-text=\"true\">       </span><span data-lexical-text=\"true\">EFG</span></p></div>"
    Received: "<div contenteditable=\"true\" style=\"user-select: text; white-space: pre-wrap; word-break: break-word;\" data-lexical-editor=\"true\"><p class=\"editor-paragraph\" dir=\"ltr\"><span data-lexical-text=\"true\">ABD</span><span class=\"PlaygroundEditorTheme__tabNode\" data-lexical-text=\"true\">       </span><span data-lexical-text=\"true\">EFG</span></p></div>"

[etrepum]

Yes, of course the tests that use tab node will have to change when a class is added.

[vantage-ola]

@etrepum I noticed something a while ago(totally unrelated) i dont know if its a bug.. sometimes when you press tab, it doesnt go to the next 4 spaces, it (folds the tab('4' spaces to '1' space), but when you do it again, it goes 4 spaces forward.. Why is that? is that the right behaviour

[vantage-ola]

i have a suggestion @etrepum instead of adding the artificial strikethroughs/underlines or both for affected tab nodes... what of a visual tab indicator like i have here in this screenshot?

[etrepum]

Regarding spacing with tabs - this is how they work. Tabs are not just wide spaces, they are variable width depending on how much space there is until the next horizontal tab stop.

https://developer.mozilla.org/en-US/docs/Web/CSS/tab-size
https://en.wikipedia.org/wiki/Tab_key

As far as displaying them visually goes, that would be a different feature and is unrelated to this issue. Some people expect that the tab character would have text decoration in this particular scenario, as if it was part of the same span as the rest of the text, at least that's what something like Google Docs or Word (the app, not the web version) would do.

I'm not aware of an HTML based editor that can display tabs with text decoration, so I don't have an example to point you towards, but this also isn't an issue that I've researched myself beyond reviewing this PR.