-
Notifications
You must be signed in to change notification settings - Fork 104
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(chip): add chip component #1981
Changes from 1 commit
f689e8b
94f0d2e
e541a8a
30a81d3
c7fc533
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<h1 style='display: flex; justify-content: space-between; align-items: center;'> | ||
<span> | ||
ebay-chip | ||
</span> | ||
<span style='font-weight: normal; font-size: medium; margin-bottom: -15px;'> | ||
DS v1.0.0 | ||
</span> | ||
</h1> | ||
|
||
A discrete highlighted value. | ||
|
||
## Examples and Documentation | ||
|
||
- [Storybook](https://ebay.github.io/ebayui-core/?path=/story/building-blocks-ebay-chip) | ||
- [Storybook Docs](https://ebay.github.io/ebayui-core/?path=/docs/building-blocks-ebay-chip) | ||
- [Code Examples](https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-chip/examples) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"requireRemap": [ | ||
{ | ||
"from": "./style.js", | ||
"to": "../../common/empty.js", | ||
"if-flag": "ebayui-no-skin" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { tagToString } from "../../../.storybook/storybook-code-source"; | ||
import { addRenderBodies } from "../../../.storybook/utils"; | ||
import Readme from "./README.md"; | ||
import Component from "./index.marko"; | ||
|
||
const Template = (args) => ({ | ||
input: addRenderBodies(args), | ||
}); | ||
|
||
export default { | ||
title: "building blocks/ebay-chip", | ||
component: Component, | ||
parameters: { | ||
docs: { | ||
description: { | ||
component: Readme, | ||
}, | ||
}, | ||
}, | ||
|
||
argTypes: { | ||
renderBody: { | ||
control: { type: "text" }, | ||
description: "Text to be displayed in the chip", | ||
}, | ||
deleteButtonLabel: { | ||
control: { type: "text" }, | ||
description: | ||
"A11y text for the delete button, also determines if delete button is shown", | ||
}, | ||
onDelete: { | ||
action: "on-delete", | ||
description: "Triggered when delete button is clicked", | ||
table: { | ||
category: "Events", | ||
defaultValue: { | ||
summary: "{ el, checked, originalEvent }", | ||
}, | ||
}, | ||
}, | ||
}, | ||
}; | ||
|
||
export const Standard = Template.bind({}); | ||
agliga marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Standard.args = { | ||
renderBody: "chip text", | ||
}; | ||
Standard.parameters = { | ||
docs: { | ||
source: { | ||
code: tagToString("ebay-chip", Standard.args), | ||
}, | ||
}, | ||
}; | ||
|
||
export const WithDelete = Template.bind({}); | ||
WithDelete.args = { | ||
renderBody: "chip text", | ||
deleteButtonLabel: "Delete", | ||
}; | ||
WithDelete.parameters = { | ||
docs: { | ||
source: { | ||
code: tagToString("ebay-chip", Standard.args), | ||
}, | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
$ const textId = component.elId("text"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if we don't allow the user to pass in a custom textid we should not do it this way but rather use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is nicer, thanks |
||
|
||
<span class="chip"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should allow spread for probably the root element. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
<span class="chip__text" id=textId> | ||
<${input.renderBody}/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops! This is what I meant to do, thanks for catching it. |
||
</span> | ||
<if(input.deleteButtonLabel)> | ||
<button | ||
type="button" | ||
class="chip__button" | ||
type="button" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have two type=buttons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha, good catch. I removed one of them. |
||
aria-label=input.deleteButtonLabel | ||
agliga marked this conversation as resolved.
Show resolved
Hide resolved
|
||
aria-describedby=textId | ||
on-click("emit", "delete") | ||
> | ||
<ebay-close-16-icon/> | ||
</button> | ||
</if> | ||
</span> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"attribute-groups": ["html-attributes"], | ||
"@*": { | ||
"targetProperty": null, | ||
"type": "expression" | ||
}, | ||
"@html-attributes": "expression", | ||
"@delete-button-label": "string" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
require("@ebay/skin/chip"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
[36m<DocumentFragment>[39m | ||
[36m<span[39m | ||
[33mclass[39m=[32m"chip"[39m | ||
[36m>[39m | ||
[36m<span[39m | ||
[33mclass[39m=[32m"chip__text"[39m | ||
[33mid[39m=[32m"s0-text"[39m | ||
[36m>[39m | ||
[0mchip text[0m | ||
[36m</span>[39m | ||
[36m</span>[39m | ||
[36m</DocumentFragment>[39m |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
[36m<DocumentFragment>[39m | ||
[36m<span[39m | ||
[33mclass[39m=[32m"chip"[39m | ||
[36m>[39m | ||
[36m<span[39m | ||
[33mclass[39m=[32m"chip__text"[39m | ||
[33mid[39m=[32m"s0-text"[39m | ||
[36m>[39m | ||
[0mchip text[0m | ||
[36m</span>[39m | ||
[36m<button[39m | ||
[33maria-describedby[39m=[32m"s0-text"[39m | ||
[33maria-label[39m=[32m"Delete"[39m | ||
[33mclass[39m=[32m"chip__button"[39m | ||
[33mtype[39m=[32m"button"[39m | ||
[36m>[39m | ||
[36m<svg[39m | ||
[33maria-hidden[39m=[32m"true"[39m | ||
[33mclass[39m=[32m"icon icon--close-16"[39m | ||
[33mfocusable[39m=[32m"false"[39m | ||
[36m>[39m | ||
[36m<defs[39m | ||
[33mdata-marko-key[39m=[32m"@defs s0-4-0"[39m | ||
[36m>[39m | ||
[36m<symbol[39m | ||
[33mid[39m=[32m"icon-close-16"[39m | ||
[33mviewBox[39m=[32m"0 0 16 16"[39m | ||
[36m>[39m | ||
[36m<path[39m | ||
[33md[39m=[32m"M2.293 2.293a1 1 0 0 1 1.414 0L8 6.586l4.293-4.293a1 1 0 1 1 1.414 1.414L9.414 8l4.293 4.293a1 1 0 0 1-1.414 1.414L8 9.414l-4.293 4.293a1 1 0 0 1-1.414-1.414L6.586 8 2.293 3.707a1 1 0 0 1 0-1.414Z"[39m | ||
[36m/>[39m | ||
[36m</symbol>[39m | ||
[36m</defs>[39m | ||
[36m<use[39m | ||
[33mxlink:href[39m=[32m"#icon-close-16"[39m | ||
[36m/>[39m | ||
[36m</svg>[39m | ||
[36m</button>[39m | ||
[36m</span>[39m | ||
[36m</DocumentFragment>[39m |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { expect, use } from "chai"; | ||
import chaiDom from "chai-dom"; | ||
import { composeStories } from "@storybook/marko/dist/testing"; | ||
import { render, fireEvent, cleanup } from "@marko/testing-library"; | ||
import * as stories from "../chip.stories"; // import all stories from the stories file | ||
const { WithDelete } = composeStories(stories); | ||
|
||
use(chaiDom); | ||
afterEach(cleanup); | ||
|
||
/** @type import("@marko/testing-library").RenderResult */ | ||
let component; | ||
|
||
describe("given delete button is present", () => { | ||
beforeEach(async () => { | ||
component = await render(WithDelete); | ||
}); | ||
|
||
describe("when button is clicked", () => { | ||
beforeEach(async () => { | ||
await fireEvent.click(component.getByRole("button")); | ||
}); | ||
|
||
it("then it emits the event with correct data", () => { | ||
expect(component.emitted("delete")).has.length(1); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import { use } from "chai"; | ||
import { composeStories } from "@storybook/marko/dist/testing"; | ||
import { snapshotHTML } from "../../../common/test-utils/snapshots"; | ||
import * as stories from "../chip.stories"; // import all stories from the stories file | ||
const { Standard, WithDelete } = composeStories(stories); | ||
const htmlSnap = snapshotHTML(__dirname); | ||
|
||
use(require("chai-dom")); | ||
|
||
it("renders default chip component", async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we rename these tests please?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also on this same line, we should group up all these tests in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, I updated test names. |
||
await htmlSnap(Standard); | ||
}); | ||
|
||
it("renders with close icon when label is present", async () => { | ||
await htmlSnap(WithDelete); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To what is
checked
referring? Should it bedeleted
instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, this was actually completely wrong, must've been a copy-paste error. I fixed it.