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

Add a babel plugin to handle the gu unit #500

Merged
merged 2 commits into from
Mar 18, 2020
Merged

Add a babel plugin to handle the gu unit #500

merged 2 commits into from
Mar 18, 2020

Conversation

bpierre
Copy link
Contributor

@bpierre bpierre commented Aug 18, 2019

See discussion: https://github.com/aragon/aragon-ui/issues/333

babel-plugin-aragon-ui

Plugin greatly inspired from babel-plugin-styled-components, and made to work with it.

What it does

Transforms this:

<div
  css={`
    width: 4gu;
    height: 4gu;
  `}
/>

Into this:

<div
  css={`
    width: 32px;
    height: 32px;
  `}
/>

Features

  • Adds the gu unit to styled component’s CSS blocks (in normal styled
    components, in the css prop, or using the css helper).

Installation

In your .babelrc, declare this plugin before styled-components:

{
  "plugins": [
    "@aragon/babel-plugin-aragon-ui",
    "styled-components"
  ]
}

To do

  • Support for text styles: -ui-text-style: title1
  • Support for theme values: -ui-theme(surfaceContent)

Note: both are using the CSS syntax for proprietary extensions, but we might as well decide to drop them, or not follow it exactly (e.g. by removing the initial -).

@bpierre bpierre requested review from sohkai and AquiGorka August 18, 2019 18:12
@@ -0,0 +1,53 @@
import { isStyled, isHelper } from './utils/detectors'

const GU = 8
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to bring this from aragonUI?
import { GU } from '@aragon/ui'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I thought about it, but it would mean either having a specific aragonUI as a dependency, or having it as a peerDependency, which doesn’t work since we also want to use it from aragonUI itself 😆

So I thought it’s just simpler to have it hardcoded. Another option would be to transform to x * GU, and add the import dynamically, but it seems a bit too much for something that should not change often, if ever.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, makes sense, tough circular dependency, we'll need to keep this in sync or we might end up with two different values for GU


## Features

- Adds the `gu` unit to styled component’s CSS blocks (in normal styled
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph sounds like the description, maybe it should go right after the h1?
Should we add some usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I need to make this a bit better, with some examples to. I also added some “To do” items in the PR description that I’ll add later.

Copy link
Contributor

@AquiGorka AquiGorka left a comment

Choose a reason for hiding this comment

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

❤️ gu

@stale stale bot added the abandoned label Sep 22, 2019
@stale stale bot closed this Sep 29, 2019
@aragon aragon deleted a comment from stale bot Sep 30, 2019
@bpierre bpierre reopened this Sep 30, 2019
@stale stale bot removed the abandoned label Sep 30, 2019
@stale stale bot added the abandoned label Oct 30, 2019
@aragon aragon deleted a comment from stale bot Oct 30, 2019
@stale stale bot removed the abandoned label Oct 30, 2019
@bpierre bpierre changed the base branch from newstyle to next November 18, 2019 16:38
@stale stale bot added the abandoned label Dec 18, 2019
@aragon aragon deleted a comment from stale bot Dec 18, 2019
@stale stale bot removed the abandoned label Dec 18, 2019
@stale stale bot added the abandoned label Jan 17, 2020
@aragon aragon deleted a comment from stale bot Jan 17, 2020
@stale stale bot removed the abandoned label Jan 17, 2020
@stale stale bot added the abandoned label Feb 16, 2020
@aragon aragon deleted a comment from stale bot Feb 16, 2020
@stale stale bot removed the abandoned label Feb 16, 2020
@stale stale bot added the abandoned label Mar 17, 2020
@Evalir Evalir removed the abandoned label Mar 17, 2020
@Evalir
Copy link
Contributor

Evalir commented Mar 17, 2020

Should we pick this up again or just close? A babel plugin for GU is quite nice but maybe doesn't align with current priorities. cc @bpierre

@aragon aragon deleted a comment from stale bot Mar 18, 2020
@bpierre
Copy link
Contributor Author

bpierre commented Mar 18, 2020

@Evalir I think we could merge it in its current state, since it is functional and separated from aragonUI. The idea of leaving this open was to have other people testing it a little bit more, but we or someone else can iterate on it in separate PRs. What do you think?

@Evalir
Copy link
Contributor

Evalir commented Mar 18, 2020

Sounds good to me! Let's merge and then iterate on it. 😬

@bpierre bpierre changed the base branch from next to master March 18, 2020 20:36
@bpierre bpierre merged commit 5cbc233 into master Mar 18, 2020
@bpierre bpierre deleted the babel-plugin branch March 18, 2020 20:38
corydickson pushed a commit to corydickson/aragon-ui that referenced this pull request Apr 7, 2020
Adds the gu unit to styled component’s CSS blocks (in normal styled components, in the css prop, or using the css helper).
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.

3 participants