Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Globally imported CLIPBOARD_OPTIONS isn't respected when [clipboardButtonComponent] or [clipboardButtonTemplate] isn't provided to the component #529

Closed
robertjajajja opened this issue Jul 1, 2024 · 2 comments · Fixed by #537 or #545

Comments

@robertjajajja
Copy link

Steps to reproduce:

  1. Clone the repo
git clone https://github.com/jfcere/ngx-markdown.git
cd ngx-markdown
yarn install
yarn run start

Notice that the demo is already configured with the global CLIPBOARD_OPTIONS provider in app.config.ts.

  1. Removing any [clipboardButtonComponent]="..." in any template will cause the clipboard button reverting to the default ClipboardButtonComponent not the globally provided component in app.config.ts.

  2. Removing [clipboardButtonTemplate] will cause the same problem.


Description of the problem

In fact, none of clipboard enabled markdown components in the demo respects the globally defined buttonComponent.

Notice MarkdownPipe doesn't suffer the same problem. e.g.

<div [innerHtml]="code | language : '...' | markdown : { clipboard: true } | async" />

will respect the globally defined buttonComponent without supplying the same buttonComponent again.

This affects every version of ngx-markdown since the introduction of ClipboardJS.


Expected behaviour: When a buttonComponent is globally provided either by provideMarkdown or MarkdownModule.forRoot(), any clipboard enabled MarkdownComponent that didn't define their [clipboardButtonComponent] nor [clipboardButtonTemplate] should use the globally provided buttonComponent.

Alternatively if said problem was actually the expected behaviour, the injection token probably should be discarded or at least the pipe should have consistent behaviour with the component.


Code analysis

In markdown.service.ts:

    if (clipboard) {
      this.renderClipboard(element, viewContainerRef, {
        ...this.DEFAULT_CLIPBOARD_OPTIONS,
        ...this.clipboardOptions,
        ...clipboardOptions,
      });
    }

Here, value of this.clipboardOptions is from dependency injection which will be the globally defined CLIPBOARD_OPTIONS provided value.

Notice when clipboardOptions is undefined, clipboardOptions isn't going to override previous spreads since { ...{ foo: 'bar' }, ...undefined } = { foo: 'bar' }. However, when the clipboardOptions is non-empty, even it's properties were empty, e.g. { buttonComponent: undefined, buttonTemplate: undefined }, spreading it to the object would cause both properties be undefined since { ...{ foo: 'bar' }, ...{ foo: undefined }} = { foo: undefined }.

The MarkdownComponent is exactly the latter case and the MarkdownPipe is the former case.

In markdown.component.ts:

    const renderOptions: RenderOptions = {
      clipboard: this.clipboard,
      clipboardOptions: {
        buttonComponent: this.clipboardButtonComponent,
        buttonTemplate: this.clipboardButtonTemplate,
      },
      katex: this.katex,
      katexOptions: this.katexOptions,
      mermaid: this.mermaid,
      mermaidOptions: this.mermaidOptions,
    };

Notice here, this.clipboardButtonComponent and this.clipboardButtonTemplate are both undefined when neither [clipboardButtonComponent] or [clipboardButtonTemplate] are provided. renderOptions is eventually passed down to MarkdownService.render():

this.markdownService.render(this.element.nativeElement, renderOptions, this.viewContainerRef);

caused the described problem.

In markdown.pipe.ts, since the options is passed in as a whole object, a consumer could simply choose to not define clipboardOptions property which avoids the described problem.

@AlexDmr
Copy link

AlexDmr commented Aug 27, 2024

Totally agree with the problem, it is currently impossible to use the global configuration in order to define a custom clipboard button.
I guess this could be debugged quite quickly, all the more since @robertjajajja did a quite deep exploration of the causes of the problem.

jfcere pushed a commit that referenced this issue Sep 30, 2024
Co-authored-by: Ondrej Kovac <ondrej.kovac@bisimulations.com>
jfcere added a commit that referenced this issue Oct 1, 2024
* Fix globally imported CLIPBOARD_OPTIONS (#529) (#537)
* Fix clipboard demo examples
* Add unit test

---------

Co-authored-by: klofi <ondra.kovac@gmail.com>
Co-authored-by: Ondrej Kovac <ondrej.kovac@bisimulations.com>
@jfcere
Copy link
Owner

jfcere commented Oct 1, 2024

Global import of CLIPBOARD_OPTIONS has been fix and released as part of ngx-markdown v18.1.0.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants