Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[feat] pfe-codeblock should have an opt out for Prismjs #2022

Open
heyMP opened this issue May 5, 2022 · 4 comments
Open

[feat] pfe-codeblock should have an opt out for Prismjs #2022

heyMP opened this issue May 5, 2022 · 4 comments
Labels
priority: medium Severity level: 2

Comments

@heyMP
Copy link
Contributor

heyMP commented May 5, 2022

Description of the issue

Some of our websites are already loading a version of PrismJS which is causing a version collision with pfe-codeblock. Ideally we'd want to have a mechanism for opting out or opting in to loading PrismJS assets. This is challenging in the fact that we call Prism js methods that are version specific in our component and we import version specific CSS into our component.

Impacted component(s)

  • pfe-codeblock

Steps to reproduce

  1. Go to https://www.ansible.com/blog/event-driven-remediation-with-systemd-and-red-hat-ansible-automation-platform
  2. Scroll down to the codeblock examples that contain yaml files
  3. You can see that they are not syntax highlighted (they are all white()
  4. Go into the network and disable webrh.webcomponents.js
  5. Refresh the page with the console still open
  6. Scroll down to the codeblock examples that contain yaml files
  7. Verify that the syntax highlliting is back

Possible Solutions

  1. Add an opt in/out method for dynamically import Prismjs dependencies per site.
  2. Upgrade Prismjs in pfe-codeblock to support YAML
@heyMP heyMP added priority: low Severity level: 3 priority: medium Severity level: 2 and removed priority: low Severity level: 3 labels May 5, 2022
@heyMP
Copy link
Contributor Author

heyMP commented May 8, 2022

In talking to the stakeholders, it looks like upgrading Prism and adding support for YAML will most likely be preferable.

@bennypowers
Copy link
Member

Might also be worth trying highlightjs which has an esm distribution

@heyMP
Copy link
Contributor Author

heyMP commented May 11, 2022

Might also be worth trying highlightjs which has an esm distribution

Nice! Definitely would prefer an esm package. We would just need to make sure we don't break any existing syntaxes.

Customer Portal uses pfe-codeblock frequently I believe.

@bennypowers
Copy link
Member

bennypowers commented May 12, 2022

The way I see it if the highlighting CSS is local to the shadow DOM, we should be g2g. We might want to expose per-token css props though, and that might affect users.

at the very least, swapping libraries will not be a pixel-perfect change, so it should probably be marked as breaking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium Severity level: 2
Projects
None yet
Development

No branches or pull requests

2 participants