[EuiMarkdownEditor] Allow more plugins to be excludable #7676

sakurai-youhei · 2024-04-11T16:14:35Z

Summary

This PR adds a way to exclude the line-breaks plugin from what getDefaultEuiMarkdownPlugins() defaults and revive the original line-breaking by two trailing spaces in markdown.

Closes #7673

QA

Go to https://eui.elastic.co/pr_7676/#/editors-syntax/markdown-editor#unregistering-plugins
Confirm that toggling on/off the various plugin switches affects the markdown output as expected

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
Docs site QA
- Added documentation
  - [ ] Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- Added or updated jest ~~and cypress tests~~
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist - N/A

Q. Without line-breaks plugin, does EuiMarkdownEditor still use "GitHub flavored markdown"?

The link points to https://guides.github.com/features/mastering-markdown/, which was changed from https://github.github.com/gfm/ in 6d94f22.
Excluding line-breaks plugin results in a line-breaking manner defined in the latter, which is "GitHub Flavored Markdown Spec Version 0.29-gfm (2019-04-06)".

So, the answer is probably YES, but it might be better to replace the link depending on line-breaks plugin. Because of this, I'm leaving the text (no reason to change) and link (potentially preferable but not mandatory) as they are. I will appreciate any comments on this point.

cee-chen · 2024-04-12T19:29:58Z

src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts

-}: { exclude?: Array<'tooltip'> } = {}): DefaultEuiMarkdownParsingPlugins => {
+}: {
+  exclude?: Array<'tooltip' | 'line-breaks'>;
+} = {}): DefaultEuiMarkdownParsingPlugins => {


[Not a blocking comment, just me thinking out loud] I wonder if we should extend the exclude list more generally to allow excluding any of the default plugins. I might spike out an attempt at this if you're cool with that!

@sakurai-youhei I'm so sorry I kind of just took over your PR 😂 d04966a

Let me know what you think of the changes! I think it makes sense to allow consumers to set configure plugins flexibly where possible.

@cee-chen Thank you for giving me such a great opportunity to learn from your code. I shrunk and limited the scope to the specific exclusion because I didn't think I could handle the full scope. The flexibility makes sense to me, too.

cee-chen · 2024-04-12T19:30:23Z

src/components/markdown_editor/plugins/markdown_default_plugins/plugins.test.ts

❤️ Thank you so much for adding a test file here and for so carefully following our QA steps!! Not all of them apply to every PR but I super appreciate the time you took to check it!

+ convert `line-breaks` plugin to camel case for casing consistency + DRY out types and exclude iteration + simplify tests slightly - IMO with static maps, there's less need to confirm the correct behavior, just that something was pushed or not pushed

cee-chen · 2024-04-13T00:02:22Z

Answering a few questions in your PR description:

The line-breaks plugin is not excluded on Code Sandbox. Is that because Code Sandbox loads the release version of eui??

Yes, CodeSandbox does not include changes made in PRs. That step usually isn't one we fill out unless we're doing extensive work on our docs - I've gone ahead and crossed it out for you (as well as the Figma step, which you are right doesn't apply here!)

Q. Without line-breaks plugin, does EuiMarkdownEditor still use "GitHub flavored markdown"?

I'm good with the decision you've made personally! I don't have a strong opinion either way and I appreciate the amount of detail you went into with your reasoning!

cee-chen · 2024-04-13T00:08:50Z

src/components/markdown_editor/plugins/markdown_default_plugins/parsing_plugins.ts

+const DEFAULT_PARSING_PLUGINS: Record<
+  ExcludableDefaultPlugins,
+  DefaultEuiMarkdownParsingPlugins[0]
+> = {
+  emoji: [emoji, { emoticon: false }],


[continuing to think out loud even more] Even more pie in the sky, but I think this map/setup also gives us the potential to allow consumers to configure the default plugin settings someday! That way if they do want emoticons for example, they could pass in a config object instead of having to turn it off completely and re-pass in the plugin.

But, that's very out of scope for this PR and no one's asking for it so I'm definitely not going to do it now 😆

cee-chen

Thanks so much again @sakurai-youhei for the super high quality EUI contribution! ✨ If my changes and the docs example look good to you feel free to merge this in once CI passes. If you have any questions or objections however, let me know and we can continue discussing!

kibanamachine · 2024-04-13T00:22:26Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7676/
Storybook: https://eui.elastic.co/pr_7676/storybook

elasticmachine · 2024-04-13T00:22:33Z

💚 Build Succeeded

Buildkite Build
Commit: f297f6f

History

💚 Build #1732 succeeded caf13dd

sakurai-youhei · 2024-04-13T10:06:14Z

@cee-chen Thank you very much. All good to me. The revised example is outstandingly awesome for understanding what each plugin does. This PR activity was a great experience for me to learn EUI, one giant leap for me. :) I appreciate your review and commits!

P.S. Since I don't seem to have the right, please click the merge button on your end.

cee-chen · 2024-04-13T19:57:05Z

P.S. Since I don't seem to have the right, please click the merge button on your end.

Haha so sorry, I always forget this part 🤦 Thanks again for your great work, I'm so glad you had a good experience! We'd love to have you again as a contributor if you ever feel like it!! 🚀

`v94.1.0-backport.0` ⏩ `v94.2.1-backport.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v94.2.1-backport.0`](https://github.com/elastic/eui/releases/v94.2.1-backport.0) **This is a backport release only intended for use by Kibana.** - Reverted the `EuiFlexGroup`/`EuiFlexItem` `component` prop feature due to Kibana typing issues ## [`v94.2.1`](https://github.com/elastic/eui/releases/v94.2.1) **Bug fixes** - Fixed an `EuiTabbedContent` edge case bug that occurred when updated with a completely different set of `tabs` ([#7713](elastic/eui#7713)) - Fixed the `@storybook/test` dependency to be listed in `devDependencies` and not `dependencies` ([#7719](elastic/eui#7719)) ## [`v94.2.0`](https://github.com/elastic/eui/releases/v94.2.0) - Updated `getDefaultEuiMarkdownPlugins()` to allow excluding the following plugins in addition to `tooltip`: ([#7676](elastic/eui#7676)) - `checkbox` - `linkValidator` - `lineBreaks` - `emoji` - Updated `EuiSelectable`'s `isPreFiltered` prop to allow passing a configuration object, which allows disabling search highlighting in addition to search filtering ([#7683](elastic/eui#7683)) - Updated `EuiFlexGroup` and `EuiFlexItem` prop types to support passing any valid React component type to the `component` prop and ensure proper type checking of the extra props forwarded to the `component`. ([#7688](elastic/eui#7688)) - Updated `EuiSearchBar` to allow the `@` special character in query string searches ([#7702](elastic/eui#7702)) - Added a new, optional `optionMatcher` prop to `EuiSelectable` and `EuiComboBox` allowing passing a custom option matcher function to these components and controlling option filtering for given search string ([#7709](elastic/eui#7709)) **Bug fixes** - Fixed an `EuiPageTemplate` bug where prop updates would not cascade down to child sections ([#7648](elastic/eui#7648)) - To cascade props down to the sidebar, `EuiPageTemplate` now explicitly requires using the `EuiPageTemplate.Sidebar` rather than `EuiPageSidebar` - Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct paddings for icon shapes set to `side: 'right'` ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore `icon`/`prepend`/`append` when `controlOnly` is set to true ([#7666](elastic/eui#7666)) - Fixed `EuiColorPicker`'s input not setting the correct right padding for the number of icons displayed ([#7666](elastic/eui#7666)) - Visual fixes for `EuiRange`s with `showInput`: ([#7678](elastic/eui#7678)) - Longer `append`/`prepend` labels no longer cause a background bug - Inputs can no longer overwhelm the actual range in width - Fixed a visual text alignment regression in `EuiTableRowCell`s with the `row` header scope ([#7681](elastic/eui#7681)) - Fixed `toolTipProps` type on `EuiSuperUpdateButton` to use `Partial<EuiToolTipProps>` ([#7692](elastic/eui#7692)) - Fixes missing prop type for `popperProps` on `EuiDatePicker` ([#7694](elastic/eui#7694)) - Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns` when moving columns to the left/right ([#7701](elastic/eui#7701)) ([#7698](elastic/eui#7698)) - Fixed `EuiSuperDatePicker` to validate date string with respect of locale on `EuiAbsoluteTab`. ([#7705](elastic/eui#7705)) - Fixed a visual bug with `EuiSuperDatePicker`'s absolute tab on small mobile screens ([#7708](elastic/eui#7708)) - Fixed i18n of empty and loading state messages for the `FieldValueSelectionFilter` component ([#7718](elastic/eui#7718)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.6.0 ([#7599](elastic/eui#7599)) - Updated `remark-rehype` to v8.1.0 ([#7601](elastic/eui#7601)) **Accessibility** - Improved `EuiBasicTable` and `EuiInMemoryTable`'s selection checkboxes to have unique aria-labels per row ([#7672](elastic/eui#7672)) - Added `aria-valuetext` attributes to `EuiRange`s with tick labels for improved screen reader UX ([#7675](elastic/eui#7675)) - Updated `EuiAccordion` to keep focus on accordion trigger instead of moving to content on click/keypress ([#7696](elastic/eui#7696)) - Added `aria-disabled` attribute to `EuiHorizontalSteps` when status is "disabled" ([#7699](elastic/eui#7699)) --------- Co-authored-by: Tomasz Kajtoch <[email protected]>

sakurai-youhei added 3 commits April 12, 2024 00:40

Add option to opt out of line-breaks plugin

7612d36

Add changelog

5f01ec1

Updated snippet

caf13dd

sakurai-youhei marked this pull request as ready for review April 12, 2024 02:52

sakurai-youhei requested a review from a team as a code owner April 12, 2024 02:52

cee-chen reviewed Apr 12, 2024

View reviewed changes

cee-chen added 2 commits April 12, 2024 14:57

Update docs & changelog

f297f6f

cee-chen reviewed Apr 13, 2024

View reviewed changes

cee-chen approved these changes Apr 13, 2024

View reviewed changes

cee-chen changed the title ~~[EuiMarkdownEditor] Add option to opt out of line-breaks plugin~~ [EuiMarkdownEditor] Allow more plugins to be excludable Apr 13, 2024

cee-chen merged commit 07d5338 into elastic:main Apr 13, 2024
7 checks passed

sakurai-youhei mentioned this pull request Apr 13, 2024

[Markdown] Consecutive lines are broken by line separators elastic/kibana#180576

Closed

cee-chen mentioned this pull request Apr 29, 2024

Upgrade EUI to v94.2.1 elastic/kibana#182023

Merged

cee-chen mentioned this pull request Jul 29, 2024

[Docs] Fix EuiMarkdownEditor plugin example polluting other examples #7929

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiMarkdownEditor] Allow more plugins to be excludable #7676

[EuiMarkdownEditor] Allow more plugins to be excludable #7676

sakurai-youhei commented Apr 11, 2024 •

edited by cee-chen

Loading

cee-chen Apr 12, 2024

cee-chen Apr 12, 2024 •

edited

Loading

sakurai-youhei Apr 13, 2024

cee-chen Apr 12, 2024

cee-chen commented Apr 13, 2024

cee-chen Apr 13, 2024

cee-chen left a comment

kibanamachine commented Apr 13, 2024

elasticmachine commented Apr 13, 2024

sakurai-youhei commented Apr 13, 2024

cee-chen commented Apr 13, 2024

[EuiMarkdownEditor] Allow more plugins to be excludable #7676

[EuiMarkdownEditor] Allow more plugins to be excludable #7676

Conversation

sakurai-youhei commented Apr 11, 2024 • edited by cee-chen Loading

Summary

QA

General checklist

Q. Without line-breaks plugin, does EuiMarkdownEditor still use "GitHub flavored markdown"?

cee-chen Apr 12, 2024

Choose a reason for hiding this comment

cee-chen Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

sakurai-youhei Apr 13, 2024

Choose a reason for hiding this comment

cee-chen Apr 12, 2024

Choose a reason for hiding this comment

cee-chen commented Apr 13, 2024

cee-chen Apr 13, 2024

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 13, 2024

elasticmachine commented Apr 13, 2024

💚 Build Succeeded

History

sakurai-youhei commented Apr 13, 2024

cee-chen commented Apr 13, 2024

sakurai-youhei commented Apr 11, 2024 •

edited by cee-chen

Loading

cee-chen Apr 12, 2024 •

edited

Loading