Consolidate EuiText and EuiMarkdownFormat styling by elizabetdev · Pull Request #4663 · elastic/eui

elizabetdev · 2021-03-24T19:47:59Z

Summary

This PR is a first step in making the EuiMarkdownFormat use the EuiText. So the EuiText improvements were thought in a way to benefit the EuiMarkdownFormat usage.

So given the complexity of this PR, we should improve issues like bottom margins of EUI components inside the EuiText in a different PR.

Or we should open a feature branch just to tackle styling issues that exist in the EuiText or in components when they live inside the EuiText. But I would say that this PR closes #4145.

Render markdown files in our docs

Probably it's not a necessary feature, but I started this attempt on 🎉 Documentation Section Examples Redesign #4449. There were still a few issues like the "scroll to" functionality not going to the exact position that it's now fixed.
The MD files are rendered with the EuiMarkdownFormat. Each h2 inside the MD files is transformed into a new section/side nav item.

More components with Remark

The EuiCodeblock was fixed to have a better wrapper.
Blockquotes now uses a specific markdown format class
Tables now use a specific markdown format class
HRs are now using the EuiHorizontalRule

Scale text mixin

The euiScaleText mixing had some changes:

It now accepts a $sizingMethod that can be em or rem.
All the font sizes are calculated using a function fontSizeToRemOrEm($size, $sizingMethod: 'rem').
All the margin and paddings are calculated using a function called marginToRemOrEm($elementSize, $elementFontSize, $sizingMethod: 'rem').
The line-height used to return rem now returns unitless numbers. This way, it behaves the same for both rem and em font-size scenarios.

EuiText

It now receives a new size relative. This works for scenarios where we want to scale the font.
We can also pass a non-named color like a hex value.
Blockquotes font size now matches the base font size which is the same as paragraphs.

EuiMarkdownFormat styles

Text, Borders, and backgrounds change according to the color passed. They can also inherit or receive any EUI named color.
With this approach, I don't see the reason for having a reverseColorsFromTheme or a colorMethod that accepts solid or translucent as an option.
The decision of the color passed is on the consumer implementation. By knowing the theme context, consumers can pass colors that make sense. Also, the color prop accepts rgba colors for translucent scenarios.

Pages to test the new functionalities:

Checklist

Check against all themes for compatibility in both light and dark modes
[ ] Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
Added documentation
~~[ ] Checked Code Sandbox works for the any docs examples~~
Added or updated jest tests
~~[ ] Checked for breaking changes and labeled appropriately~~
~~[ ] Checked for accessibility including keyboard-only and screenreader modes~~
A changelog entry exists and is marked appropriately

kibanamachine · 2021-03-24T20:12:46Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

kibanamachine · 2021-03-25T13:18:02Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

…-and-EuiText

kibanamachine · 2021-05-28T13:11:28Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

kibanamachine · 2021-05-28T15:07:16Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

…-and-EuiText

kibanamachine · 2021-06-15T17:07:33Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

…-and-EuiText

kibanamachine · 2021-06-23T12:16:13Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

kibanamachine · 2021-06-23T15:19:29Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

kibanamachine · 2021-06-24T16:26:54Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

…-and-EuiText

kibanamachine · 2021-07-15T18:06:52Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

cchaos

❤️ I know this was a lot of work, especially the relative sizing and custom coloring. I see a lot of duplicated mixin/functions but I'm not to worried about "DRY" code at the moment because I'm really hoping Emotion will make this easier in the near future.

There are definitely a few changes to the margins/paddings/line-heights of certain text elements. It would be great to track these in a comment so that we can show the intentionality of the changes and to know exactly what has changed.

One specific formatting thing that I'm seeing with the Markdown formatter, is that is only seems to be wrapping multi-line code blocks with our actual EuiCodeBlock (not inline). It would be great that even just single line code blocks could be wrapped in EuiCodeBlock.

src/themes/eui-amsterdam/global_styling/variables/_typography.scss

src/components/text/text_color.tsx

src/components/text/text.tsx

src-docs/src/views/markdown_editor/markdown_format_sink.js

src-docs/src/views/markdown_editor/mardown_format_example.js

src-docs/src/routes.js

src-docs/src/components/guide_page/_guide_page.scss

thompsongl · 2021-07-19T21:50:34Z

src/components/markdown_editor/plugins/markdown_default_plugins.tsx

          ) : (
            <EuiCode {...props} />
          ),
+        // When we use block code "fences" the code tag is replaced by the `EuiCodeBlock`.


I don't expect any trouble, but for visibility, #4970 also edited this file and will likely merge first.

…-and-EuiText

kibanamachine · 2021-07-20T14:39:08Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

kibanamachine · 2021-07-20T15:49:13Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

elizabetdev · 2021-07-21T12:23:05Z

@cchaos I went through your comments and I fixed all of them expect the following one:

One specific formatting thing that I'm seeing with the Markdown formatter, is that is only seems to be wrapping multi-line code blocks with our actual EuiCodeBlock (not inline). It would be great that even just single line code blocks could be wrapped in EuiCodeBlock.

Right now this part of the code checks if there are linebreaks to use code block or otherwise inline code.

eui/src/components/markdown_editor/plugins/markdown_default_plugins/processing_plugins.tsx

Lines 64 to 70 in 004ac12

    
           code: (props: any) => 
        
             // If there are linebreaks use codeblock, otherwise code 
        
             /\r|\n/.exec(props.children) ? ( 
        
               <EuiCodeBlock fontSize="m" paddingSize="s" {...props} /> 
        
             ) : ( 
        
               <EuiCode {...props} /> 
        
             ),

I can use the technique mentioned here: rehypejs/rehype-react#6 (comment). If the props contain a className like className='language-js' is a codeblock, otherwise inline code.

code: (props) => {
          return props.className ? (
            <EuiCodeBlock fontSize="m" paddingSize="s" isCopyable {...props} />
          ) : (
            <EuiCode {...props} />
          );
        },

But I tried and if consumers don't pass a language like ```js and just pass triple backticks ``` no className is created and with this technique, it assumes it's an inline code and used the <EuiCode {...props} />. So consumers would always have to specify a language when using triple backticks.

So not sure how can we better solve this issue. Ideally, if it's passed triple backticks with or without a language it should use a EuiCodeBlock otherwise EuiCode.

But I couldn't find better solutions. @thompsongl any idea on how we can solve this?

cchaos · 2021-07-21T12:48:43Z

Does that mean that rehype-react doesn't support differing between triple backticks and single backticks?

elizabetdev · 2021-07-21T13:29:28Z

I think the problem starts before. When the code gets to rehype-react the node it's already transformed without many options to distinguish if it is inline code or not.

This seems a good solution: rehypejs/rehype-react#6 (comment). Somehow we need to inject a class property at the node in order to distinguish inline/fenced code. So when it gets to rehype-react we know it's inline or not.

thompsongl · 2021-07-21T17:16:30Z

Latest commit fixes the inline (single tick) vs. fenced (triple tick) code block discrepancy.
In reality, it had more to do with whether language was provided for a fenced block. Regardless, we now get the appropriate code type by making the distinction clear in remark-prismjs.

kibanamachine · 2021-07-21T17:36:45Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

cchaos

💯 LGTM! I still think it's a good idea to track the differences in the text element renders. For instance the blockquote text size has been reduced. (I think it looks better, but others might not realize).

Maybe you can also update the home_view link to the new Getting Started page? 😉

I'd like an engineer to do a quick pass and then don't forget to merge with upstream to get changelog changes.

snide · 2021-07-22T12:23:58Z

cc @gjones You'll want to watch for this one coming downstream after it merges. We utilize the EuiText side a bit in docs.

kibanamachine · 2021-07-22T12:39:11Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

…-and-EuiText

elizabetdev · 2021-07-22T12:53:28Z

@cchaos I confirmed if all the margins, paddings, line-heights and font-sizes were matching the old sizes: https://github.com/elastic/eui/blob/master/src/components/text/_text.scss

It seems that I only changed blockquotes to match the base font-size/line-height. I added a CL entry with this change and updated the PR summary.

kibanamachine · 2021-07-22T13:11:03Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_4663/

thompsongl

Code changes LGTM! Thanks for all the work on this 🙏

elizabetdev · 2021-07-22T15:02:30Z

Thanks @thompsongl and @cchaos for the help! 🎉

elizabetdev added 3 commits March 24, 2021 19:24

Wrapping the EuiMarkdownFormat content with EuiText

514ab4d

Comments

c0d4046

Unnecessary comment

e84df00

Initial markdown format styles example

f002a89

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

6d5133a

…-and-EuiText

elizabetdev added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label May 28, 2021

Getting started page matching other guideline pages

c831eea

elizabetdev added 4 commits May 28, 2021 16:14

wip

baf8dbe

Improving euiScaleText mixin to also return em sizes

65c3b6f

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

bb1e1de

…-and-EuiText

Better amsterdam scaling

4d45e35

elizabetdev added 6 commits June 18, 2021 13:00

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

76eeb28

…-and-EuiText

Move mixins to appropriate files. Renaming mixins.

537ca0a

Just one EM class

6f3249c

Moving functions to typography variables file

23f1f8a

marginToRemOrEm

8c42aae

Adding relative class name and inherit text color

a4739a5

Fixing jest test

45f17a0

Ghost colors

b547732

elizabetdev added 3 commits July 1, 2021 14:43

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

df6fd71

…-and-EuiText

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

8e52331

…-and-EuiText

Adding components theme

6ca83ef

cchaos suggested changes Jul 19, 2021

View reviewed changes

thompsongl reviewed Jul 19, 2021

View reviewed changes

elizabetdev added 2 commits July 20, 2021 14:59

Addressing PR review

344d9f8

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

8efb4a5

…-and-EuiText

elizabetdev added 2 commits July 20, 2021 16:02

Better text styles for text color

844bfdc

Adding CSSProperties['color'] type

aa6ae66

retain fenced block styles

9f95d59

cchaos approved these changes Jul 21, 2021

View reviewed changes

thompsongl self-requested a review July 21, 2021 19:16

elizabetdev added 2 commits July 22, 2021 12:36

Adding home page getting started link

74986ac

Updating CL

678cb90

elizabetdev added 3 commits July 22, 2021 13:40

Update CL

fbf7ced

Merge remote-tracking branch 'upstream/master' into EuiMarkdownFormat…

f1aa79d

…-and-EuiText

CL again :D

eeea91f

thompsongl approved these changes Jul 22, 2021

View reviewed changes

elizabetdev merged commit 3e4beeb into elastic:master Jul 22, 2021

chandlerprall mentioned this pull request Jul 29, 2021

Upgrade EUI to v36.1.0 elastic/kibana#107231

Merged

Conversation

elizabetdev commented Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Render markdown files in our docs

More components with Remark

Scale text mixin

EuiText

EuiMarkdownFormat styles

Pages to test the new functionalities:

Checklist

Uh oh!

kibanamachine commented Mar 24, 2021

Uh oh!

kibanamachine commented Mar 25, 2021

Uh oh!

kibanamachine commented May 28, 2021

Uh oh!

kibanamachine commented May 28, 2021

Uh oh!

kibanamachine commented Jun 15, 2021

Uh oh!

kibanamachine commented Jun 23, 2021

Uh oh!

kibanamachine commented Jun 23, 2021

Uh oh!

kibanamachine commented Jun 24, 2021

Uh oh!

kibanamachine commented Jul 15, 2021

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thompsongl Jul 19, 2021

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jul 20, 2021

Uh oh!

kibanamachine commented Jul 20, 2021

Uh oh!

elizabetdev commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cchaos commented Jul 21, 2021

Uh oh!

elizabetdev commented Jul 21, 2021

Uh oh!

thompsongl commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Jul 21, 2021

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

snide commented Jul 22, 2021

Uh oh!

kibanamachine commented Jul 22, 2021

Uh oh!

elizabetdev commented Jul 22, 2021

Uh oh!

kibanamachine commented Jul 22, 2021

Uh oh!

thompsongl left a comment

Choose a reason for hiding this comment

Uh oh!

elizabetdev commented Jul 22, 2021

Uh oh!

Reviewers

Assignees

elizabetdev commented Mar 24, 2021 •

edited

Loading

elizabetdev commented Jul 21, 2021 •

edited

Loading

thompsongl commented Jul 21, 2021 •

edited

Loading