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

docs(storybook): Lemon UI #9426

Merged
merged 27 commits into from
Apr 27, 2022
Merged

docs(storybook): Lemon UI #9426

merged 27 commits into from
Apr 27, 2022

Conversation

Twixes
Copy link
Collaborator

@Twixes Twixes commented Apr 15, 2022

Problem

We have a growing suite of building block components designed by Chris (often prefixed with Lemon to differentiate them from Ant Design used before). Those components serve our needs well, but the docs for using them are scattered in Storybook, and a bit lacking in examples for more advanced uses.

Changes

Adds section "Lemon UI" to Storybook, moves some existing stories there, also adding some new ones (for buttons and tables most importantly).

Screen Shot 2022-04-15 at 13 09 50

@Twixes Twixes marked this pull request as ready for review April 26, 2022 12:40
@Twixes
Copy link
Collaborator Author

Twixes commented Apr 26, 2022

This is the collection at the moment:
Lemons

It's still missing a few components:

  • LemonRow
  • LemonButtonWithSideAction
  • LemonButtonWithPopup
  • LemonButton
  • More
  • PaginationControl

And I haven't touched two stories here that could use a big revamp:

  • LemonTable
  • Popup

But this is already a huge improvement, so I think it's ready for review.

I refactored a few Lemon things, which inflates the diff:

  • Renamed LemonSpacer to LemonDivider and moved it out of LemonRow.tsx to LemonDivider.tsx
  • Moved most of disabled styling from LemonRow-based components to LemonRow itself – since it was all basically the same and maintaining separate .LemonCheckbox--disabled, .LemonSwitch--disabled, .LemonInput--disabled didn't make a ton sense
  • Fixed LemonSwitch taking up full width by default
  • Consolidated styles to the BEM convention in AlertMessage, Spinner, and Splotch
  • Removed deprecated InfoMessage, which was just AlertMessage with type="info"
  • Made LemonSwitch use Spinner in loading state for better consistency and simpler code
  • Made number handling in Lettermark more logical

Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM

Big step forward in discoverability! I didn't know LemonSpacer existed (and LemonDivider is a much better name)

We could add a linter to warn when using the components where someone should use the Lemon version instead...

@pauldambra
Copy link
Member

Example of custom linting #8156

Copy link
Contributor

@clarkus clarkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good start. I'd like to work on dialing in some color usage at some point. We need to use alternate values for text in a lot of scenarios, so mapping those to variables and guidelines would be good. I will work on examples for figma to help come up with some guidelines.

Screen Shot 2022-04-26 at 9 37 16 AM

As an example, this color combination is really hard to read. There's a related pattern with our letter-marks for identifying users and organizations. Some reusable set of color combinations for these components would be great.

@Twixes Twixes requested a review from lottiecoxon April 26, 2022 17:06
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! 👍

Also, looks like code.

@Twixes
Copy link
Collaborator Author

Twixes commented Apr 27, 2022

@pauldambra Linter rules are super cool, but my worry here is that it would also alert on all existing uses of components we're phasing out, which would be a ton of noise. So going just for dev awareness for now.

@Twixes Twixes merged commit 58b59c4 into master Apr 27, 2022
@Twixes Twixes deleted the lemon-design branch April 27, 2022 10:20
fuziontech added a commit that referenced this pull request Apr 28, 2022
* master: (137 commits)
  feat(cohorts): add cohort filter grammars (#9540)
  feat(cohorts): Backwards compatibility of groups and properties (#9462)
  perf(ingestion): unsubscribe from buffer topic while no events are produced to it (#9556)
  fix: Fix `Loading` positioning and `LemonButton` disabled state (#9554)
  test: Speed up backend tests (#9289)
  fix: LemonSpacer -> LemonDivider (#9549)
  feat(funnels): Highlight significant deviations in new funnel viz (#9536)
  docs(storybook): Lemon UI (#9426)
  feat: add support for list of teams to enable the conversion buffer for (#9542)
  chore(onboarding): cleanup framework grid experiment (#9527)
  fix(signup): domain provisioning on cloud (#9515)
  chore: split out async migrations ci (#9539)
  feat(ingestion): enable json ingestion for self-hosted by default (#9448)
  feat(cohort): add all cohort filter selectors to Storybook (#9492)
  feat(ingestion): conversion events buffer consumer (#9432)
  ci(run-backend-tests): remove CH version default (#9532)
  feat: Add person info to events (#9404)
  feat(ingestion): produce to buffer partitioned by team_id:distinct_id (#9518)
  fix: bring latest_migrations.manifest up to date (#9525)
  chore: removes unused feature flag (#9529)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants