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

feat(#5): Add Button component #11

Merged
merged 14 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/download-built-package/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ runs:
with:
name: dist
path: ${{ env.dist }}
- name: 'Install dependencies'
shell: 'bash'
run: pnpm install --force
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't really dived deep into the more special CI setup you and @NullVoxPopuli have in use for GH repos here (forms is still on blueprint defaults), so leaving the final assessment to you guys. But it seems to me if the intend was to do less by reusing build artefacts across jobs, then we loose much (all?) of this benefit when forcing to install all dependencies again, don't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Some background:

I was talking to @NullVoxPopuli about this yesterday as I was hitting an issue where the lint step (which runs glint) was failing for the test-app. We essentially need the dist directory to be able to successfully run lint, or else we get type errors when running glint. Here's an example: https://github.com/CrowdStrike/ember-toucan-core/actions/runs/4009103591

Talking to @NullVoxPopuli , the reason we need to re-install dependencies is due to pnpm. We need to re-run pnpm i to re-sync the injected links. We do the same thing over in ember-headless-table (https://github.com/CrowdStrike/ember-headless-table/blob/f08ff968d862d2267a3b2f6269116806549d51b0/.github/actions/download-built-package/action.yml#L11).

I do agree with you though, that this feels like we add additional time to each step using this action by forcing dependencies to re-install. Maybe there's an optimization we can make in the future though? I'm also completely new to pnpm, so I don't have much context other than these convos! πŸ™‡

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. I guess this is this inject thing that Preston told me also about...

Yeah, ok. Unfortunate, but hopefully this will be sorted out in the future somehow...

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli Jan 26, 2023

Choose a reason for hiding this comment

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

Yeah, hoping we can work out a plan of attack with @zkochan (pnpm owner) with making injected auto-sync.
My current best idea is to analyze package.json#files/exports for folders, and symlink those, so that files automatically sync, and we don't need the extra pnpm i.

Related:

26 changes: 13 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,6 @@ jobs:
- uses: actions/checkout@v3
- uses: ./.github/actions/pnpm

lint:
name: Lint
runs-on: ubuntu-latest
needs: [install_dependencies]
steps:
- uses: actions/checkout@v3
- uses: ./.github/actions/pnpm
- name: Lint
run: pnpm lint
- name: Formatting
run: pnpm format:check


build:
name: Build Tests
needs: [install_dependencies]
Expand All @@ -46,6 +33,19 @@ jobs:
- uses: ./.github/actions/pnpm
- uses: ./.github/actions/assert-build

lint:
name: Lint
runs-on: ubuntu-latest
needs: [install_dependencies, build]
steps:
- uses: actions/checkout@v3
- uses: ./.github/actions/pnpm
# To be able to run glint, we need the dist directory
- uses: ./.github/actions/download-built-package
- name: Lint
run: pnpm lint
- name: Formatting
run: pnpm format:check

typecheck:
name: '${{ matrix.typescript-scenario }}'
Expand Down
3 changes: 3 additions & 0 deletions docs-app/ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ const isProduction = () => EmberApp.env() === 'production';

module.exports = function (defaults) {
let app = new EmberApp(defaults, {
autoImport: {
watchDependencies: ['@crowdstrike/ember-toucan-core'],
},
'ember-cli-babel': {
enableTypeScriptTransform: true,
},
Expand Down
5 changes: 3 additions & 2 deletions docs-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-qunit": "^7.3.1",
"fractal-page-object": "^0.3.0",
"loader.js": "^4.7.0",
"postcss": "^8.4.17",
"postcss-import": "^15.0.0",
Expand All @@ -118,20 +119,20 @@
},
"dependencies": {
"@crowdstrike/ember-oss-docs": "^1.1.0",
"@crowdstrike/ember-toucan-core": "workspace:../ember-toucan-core",
"@ember/test-waiters": "^3.0.2",
"@embroider/router": "^1.9.0",
"dompurify": "^2.4.0",
"ember-browser-services": "^4.0.3",
"ember-cached-decorator-polyfill": "^1.0.1",
"ember-modifier": "^3.2.7",
"ember-resources": "^5.4.0",
"ember-toucan-core": "workspace:../ember-toucan-core",
"highlight.js": "^11.6.0",
"highlightjs-glimmer": "^1.4.1",
"tracked-built-ins": "^3.1.0"
},
"dependenciesMeta": {
"ember-toucan-core": {
"@crowdstrike/ember-toucan-core": {
"injected": true
}
}
Expand Down
15 changes: 15 additions & 0 deletions docs/components/button/demo/base-demo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
```hbs template
<Button @onClick={{this.onClick}} @variant='secondary'>Button</Button>
Copy link
Contributor Author

@ynotdraw ynotdraw Jan 21, 2023

Choose a reason for hiding this comment

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

This Button instance actually has a collision with the Button component from @crowdstrike/ember-oss-docs. For now, we can continue to write docs, but be aware you may hit this if there are components named the same there.

The eventual goal is to:

  • Release this as a package
  • Update ember-oss-docs to use this repo

A bit of a πŸ” and πŸ₯š problem

```

```js component
import Component from '@glimmer/component';
import { action } from '@ember/object';

export default class extends Component {
@action
onClick(e) {
alert('Button clicked!');
}
}
```
152 changes: 152 additions & 0 deletions docs/components/button/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
# Button

Buttons are clickable elements used primarily for actions. Button content expresses what action will occur when the user interacts with it.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not a writer, so appreciate any feedback in this entire document. I did my best tho ☺️

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good to me πŸ™ƒ

Copy link
Contributor

Choose a reason for hiding this comment

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

Unlike our internal docs, which focus on all disciplines at once, these "core" docs, are primarily focused on developers.


## Variants

You can customize the appearance of the button with the `@variant` component argument.

<div class="flex gap-x-4">
<Button @variant="primary">Primary</Button>
<Button @variant="secondary">Secondary</Button>
<Button @variant="destructive">Destructive</Button>
<Button @variant="link">Link</Button>
<Button @variant="quiet">Quiet</Button>
<Button @variant="bare">Bare</Button>
</div>

## Handling Clicks

To handle click events use the `@onClick` component argument.

```hbs
<Button @onClick={{this.handleClick}}>Click Me</Button>
```

## Disabled State

`aria-disabled` is used over the `disabled` attribute so that screenreaders can still focus the element. To set the button as disabled, use `@isDisabled`.

```hbs
<Button @isDisabled={{true}}>Disabled</Button>
```

A disabled named block is provided so that users can optionally render additional content when the button is disabled.

```hbs
<Button @isDisabled={{true}}>
<:disabled>
<svg
Copy link
Contributor Author

@ynotdraw ynotdraw Jan 21, 2023

Choose a reason for hiding this comment

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

The icon I used in this example is from an MIT-licensed open source library I built. We can replace it at any time, as we see fit.

class='h-4 w-4'
xmlns='http://www.w3.org/2000/svg'
width='24'
height='24'
stroke='currentColor'
viewBox='0 0 24 24'
>
<path
d='M18.644 21h-13.2a.945.945 0 01-1-1v-7.2a.945.945 0 011-1h13.1a.945.945 0 011 1V20a.808.808 0 01-.225.725.966.966 0 01-.675.275zm-10.9-9.2V7.3a4.3 4.3 0 118.6 0v4.5m-4.3 3.7v2'
fill='none'
stroke-linecap='round'
stroke-linejoin='round'
stroke-width='2'
/>
</svg>
</:disabled>
<:default>
Disabled
</:default>
</Button>
```

<div class="flex gap-x-4">
{{#each (array "primary" "secondary" "destructive" "link" "quiet" "bare") as |variant|}}
<Button @variant={{variant}} @isDisabled={{true}}>
<:disabled>
<svg
class='h-4 w-4'
xmlns='http://www.w3.org/2000/svg'
width='24'
height='24'
stroke='currentColor'
viewBox='0 0 24 24'
>
<path
d="M18.644 21h-13.2a.945.945 0 01-1-1v-7.2a.945.945 0 011-1h13.1a.945.945 0 011 1V20a.808.808 0 01-.225.725.966.966 0 01-.675.275zm-10.9-9.2V7.3a4.3 4.3 0 118.6 0v4.5m-4.3 3.7v2"
fill='none'
stroke-linecap='round'
stroke-linejoin='round'
stroke-width='2'
/>
</svg>
</:disabled>
<:default>
{{variant}}
</:default>
</Button>
{{/each}}
</div>

## Loading State

Button exposes an `@isLoading` component argument. The button content will be only visible to screenreaders.

```hbs
<Button @isLoading={{true}}>Loading…</Button>
```

A loading named block is also provided for providing custom loading content.

```hbs
<Button @isLoading={{true}}>
<:loading>
<svg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same comment as above in regards to icons here

class='h-4 w-4 animate-spin'
xmlns='http://www.w3.org/2000/svg'
width='24'
height='24'
stroke='currentColor'
viewBox='0 0 24 24'
>
<path
d='M5.95 5.7L7 6.75 8.05 7.8m8.4 8.4l.95.95.95.95m.2-12.4L17.5 6.75 16.45 7.8M6.35 12h-3.1m17.5 0h-2.6m-5.9 9v-3.1m0-14.9v3.1'
fill='none'
stroke-linecap='round'
stroke-linejoin='round'
stroke-width='2'
/>
</svg>
</:loading>
<:default>
Loading…
</:default>
</Button>
```

<div class="flex gap-x-4">
{{#each (array "primary" "secondary" "destructive" "link" "quiet" "bare") as |variant|}}
<Button @variant={{variant}} @isLoading={{true}}>
<:loading>
<svg
class='h-4 w-4 animate-spin'
xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
stroke="currentColor"
viewBox="0 0 24 24"
>
<path
d="M5.95 5.7L7 6.75 8.05 7.8m8.4 8.4l.95.95.95.95m.2-12.4L17.5 6.75 16.45 7.8M6.35 12h-3.1m17.5 0h-2.6m-5.9 9v-3.1m0-14.9v3.1"
fill="none"
stroke-linecap="round"
stroke-linejoin="round"
stroke-width="2"
/>
</svg>
</:loading>
<:default>
{{variant}}
</:default>
</Button>
{{/each}}
</div>
1 change: 0 additions & 1 deletion docs/demos/demo-a.md

This file was deleted.

13 changes: 12 additions & 1 deletion ember-toucan-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@
},
"peerDependencies": {
"@crowdstrike/ember-toucan-styles": "^1.0.5",
"@ember/test-helpers": "^2.8.1",
"@glimmer/tracking": "^1.1.2",
"autoprefixer": "^10.0.2",
"ember-source": "^4.8.0",
"fractal-page-object": "^0.3.0",
"postcss": "^8.2.14",
"tailwindcss": "^2.2.15 || ^3.0.0"
},
"dependencies": {
"@babel/runtime": "^7.20.7",
"@embroider/addon-shim": "^1.0.0"
},
"devDependencies": {
Expand All @@ -50,6 +53,7 @@
"@babel/plugin-syntax-decorators": "^7.17.0",
"@babel/preset-typescript": "^7.18.6",
"@crowdstrike/ember-toucan-styles": "^1.0.5",
"@ember/test-helpers": "^2.8.1",
"@embroider/addon-dev": "^2.0.0",
"@glimmer/component": "^1.1.2",
"@glimmer/tracking": "^1.1.2",
Expand Down Expand Up @@ -88,6 +92,7 @@
"eslint-plugin-n": "^15.6.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^4.0.0",
"fractal-page-object": "^0.3.0",
"postcss": "^8.2.14",
"prettier": "^2.8.3",
"prettier-plugin-ember-template-tag": "^0.3.0",
Expand All @@ -107,18 +112,24 @@
"version": 2,
"type": "addon",
"main": "addon-main.cjs",
"app-js": {}
"app-js": {
"./components/button.js": "./dist/_app_/components/button.js"
}
},
"exports": {
".": "./dist/index.js",
"./*": {
"types": "./dist/*.d.ts",
"default": "./dist/*.js"
},
"./test-support": "./dist/test-support/index.js",
"./addon-main.js": "./addon-main.cjs"
},
"typesVersions": {
"*": {
"test-support": [
"./dist/test-support/index.d.ts"
],
"*": [
"dist/*"
]
Expand Down
1 change: 1 addition & 0 deletions ember-toucan-core/rollup.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default {
'components/**/*.js',
'index.js',
'template-registry.js',
'test-support/index.js',
]),

// These are the modules that should get reexported into the traditional
Expand Down
19 changes: 19 additions & 0 deletions ember-toucan-core/src/components/button.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<button
aria-disabled={{if @isDisabled "true"}}
class={{this.styles}}
type="button"
{{on "click" this.onClick}}
...attributes
>
{{#if @isLoading}}
{{yield to="loading"}}
<span class="sr-only" data-loading>{{yield}}</span>
{{else if @isDisabled}}
<span class="flex items-center gap-x-2">
{{yield}}
{{yield to="disabled"}}
</span>
{{else}}
{{yield}}
{{/if}}
</button>
Loading