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

react: styling improvements for gherkin components #1391

Merged
merged 124 commits into from
May 25, 2021

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented Feb 27, 2021

Summary

Improve styling and CSS architecture of gherkin components in @cucumber/react.

Introduce an MDG component for rendering Markdown features.

Details

Styles are colocated and encapsulated with presentational components via (S)CSS modules. This means we are now producing processed CSS output via webpack in the html-formatter build, not just copying some stylesheets from react - this is reflected in some makefile and code changes.

An optional CustomRendering component enables customisation of styles via providing own CSS classes, or even overriding of React rendering. This is intentionally for a small subset of presentational
components at this point.

Theming is enabled via CSS custom properties - we could conceivably let people set these via --formatOptions.

More detail in the updated README.md. There are also new stories in Storybook illustrating what's possible with the customisation.

Motivation and Context

  • Improve general look and feel
  • Improve component encapsulation
  • Allow incremental customisation/theming

Screenshots (if appropriate):

118369710-5590a300-b59c-11eb-9f72-a626c97ea4ea

You can preview here in Storybook form:
https://serene-wilson-f8fc77.netlify.app/

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • The change has been ported to Java.
  • The change has been ported to Ruby.
  • The change has been ported to JavaScript.
  • The change has been ported to Go.
  • The change has been ported to .NET.
  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the CHANGELOG accordingly.

@aurelien-reeves
Copy link
Contributor

What would you think about the possibility to propose several styles through optional themes?

@davidjgoss
Copy link
Contributor Author

@aurelien-reeves yes definitely, that should be nice and easy with custom properties.

I’ll run an example up today and work on the CSS modularisation too.

@aslakhellesoy raised this in lieu of an issue, hope that’s okay?

@davidjgoss
Copy link
Contributor Author

davidjgoss commented Mar 4, 2021

I've added CSS modules now - see DocString for initial example. Once rolled out this will mean we no longer to maintain a global stylesheet as each component will have its own styles colocated, and encapsulated at build time (I'll need to update html-formatter to handle this with webpack as well).

Customisation comes from CSS custom properties, which could be set from a custom stylesheet, plus built-in themes can be activated via the <CucumberTheme> component (for example in html-formatter we could get this from a CLI option).

Inevitable dark theme:

image

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented Mar 4, 2021

OMG this looks awesome! 😍 And very clean scss + minimal logic.

I'd love to make it obvious to consumers of @cucumber/react to take complete control over the styling.

I've had an idea about a styling system/API. It's a inspired by React-Markdown Custom Renderers. A custom renderer would typically use a component from the library, and pass custom class attributes. That way, a TailwindCSS renderer would add a bunch of tailwind classes. Another one could just add a simple class name and do the styling in a custom stylesheet. Each to their own.

I think it's important to get this styling story straight. That way it will become much easier to adopt and tailor to users' needs.

LMK what you think

@davidjgoss
Copy link
Contributor Author

@aslakhellesoy yep I know what you mean. If we could make that renderer a prop of <CucumberTheme> then potentially we could handle it internally with a context. I'll have a play over the weekend and see what I can do.

@davidjgoss davidjgoss marked this pull request as ready for review May 20, 2021 00:37
@aslakhellesoy
Copy link
Contributor

I accidentally edited the summary in this PR thinking I was editing #1564 - I've done my best to restore it (reverting the edits wasn't possible).

@aslakhellesoy
Copy link
Contributor

I've made some more improvements to Markdown. The Markdown rendering and AST rendering is now very similar:

Markdown:
Screenshot 2021-05-24 at 13 08 46

Gherkin:
Screenshot 2021-05-24 at 13 09 29

@aslakhellesoy
Copy link
Contributor

@davidjgoss do you think we can merge this to main now?

@davidjgoss
Copy link
Contributor Author

@aslakhellesoy yep I'm happy if you are.

@aslakhellesoy
Copy link
Contributor

Let's do it! I'll create a new issue with various remaining styling niggles I'd like to address.

I'll merge when the CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants