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

Allowed passing @tagName for dynamic tag #46

Merged
merged 5 commits into from
Jul 10, 2020
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
32 changes: 24 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,34 +114,50 @@ The addon provides 1 Glimmer component and 3 helpers:
<details>
<summary><code>&lt;ContainerQuery&gt;</code></summary>

The component uses `...attributes` so that you can pass `class` or [`local-class`](https://github.com/salsify/ember-css-modules) _for styling_.<sup>1</sup>
### Arguments

It also accepts these arguments:
You can pass these arguments to the component.

| Name | Required | Description | Type |
|--|:--:|--|--|
| @features | Yes<sup>2</sup> | Container query definitions | POJO |
| @features | Yes<sup>1</sup> | Container query definitions | POJO |
| @dataAttributePrefix | No | Prefix for data attributes | string |
| @debounce | No | Debounce time for resize (ms) | number ≥ 0 |
| @tagName | No | Container tag name<sup>2</sup> | HTML tag name |

The component returns a few values that you can consume:
<sup>1. The component renders without error when `@features` isn't provided. In practice, you will always want to set `@features`.</sup>

<sup>2. By default, the component is a `<div>` element. You can pass a valid HTML tag name to facilitate accessibility and semantic HTML.</sup>

### Attributes

You _may_<sup>1</sup> pass attributes to the component for these reasons:

- Style (e.g. `class`, [`local-class`](https://github.com/salsify/ember-css-modules))
- Accessibility (e.g. ARIA attributes<sup>2</sup>, roles)

<sup>1. Do refrain from overusing splattributes (e.g. pass a `{{did-insert}}` modifier to fetch data), since the component's API may change and cause unexpected results. Practice separation of concerns when possible. For example, data fetching can be handled by another element or [`@use` decorator](https://github.com/emberjs/rfcs/blob/use-and-resources/text/0567-use-and-resources.md).</sup>

<sup>2. When an [ARIA attribute has multiple values](https://github.com/ijlee2/ember-container-query/issues/38#issuecomment-647017665), the order of values can matter. At the moment, splattributes doesn't guarantee the order.</sup>

### Outputs

You can consume these values in your app or addon.

| Name | Yielded | Description | Type |
|--|:--:|--|--|
| features | Yes | Container query results | POJO |
| dimensions | Yes | Container dimensions | POJO |
| data-container-query-_{featureName}_ | No | Data attributes for CSS selector | HTML data attribute |

<sup>1. Do refrain from overusing splattributes (e.g. pass a `{{did-insert}}` modifier to fetch data), since the component's API may change and cause unexpected results. Practice separation of concerns when possible. For example, data fetching can be handled by another element or [`@use` decorator](https://github.com/emberjs/rfcs/blob/use-and-resources/text/0567-use-and-resources.md).</sup>

<sup>2. The component renders without error when `@features` isn't provided. In practice, you will always want to set `@features`.</sup>

</details>


<details>
<summary><code>{{cq-aspect-ratio}}</code>, <code>{{cq-height}}</code>, <code>{{cq-width}}</code></summary>

### Arguments

All helpers accept these arguments:

| Name | Required | Description | Type |
Expand Down
34 changes: 18 additions & 16 deletions addon/components/container-query.hbs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<div
class="container-query"
data-test-container-query
{{did-insert this.queryContainer}}
{{did-resize this.queryContainer debounce=this.debounce}}
...attributes
>
{{yield (hash
features=this.queryResults
dimensions=(hash
aspectRatio=this.aspectRatio
height=this.height
width=this.width
)
)}}
</div>
{{#let (element this.tagName) as |Tag|}}
<Tag
class="container-query"
data-test-container-query
{{did-insert this.queryContainer}}
{{did-resize this.queryContainer debounce=this.debounce}}
...attributes
>
{{yield (hash
features=this.queryResults
dimensions=(hash
aspectRatio=this.aspectRatio
height=this.height
width=this.width
)
)}}
</Tag>
{{/let}}
7 changes: 7 additions & 0 deletions addon/components/container-query.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ export default class ContainerQueryComponent extends Component {
return this.args.debounce ?? 0;
}

constructor() {
super(...arguments);

// The dynamic tag is restricted to be immutable
this.tagName = this.args.tagName || 'div';
Copy link
Owner Author

Choose a reason for hiding this comment

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

I used the OR operator || so that, in addition to null and undefined (which can be covered by the nullish coalescing operator ??), the empty string '' would also result in the default <div> tag.

Copy link

Choose a reason for hiding this comment

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

Your note on setting this in the constructor and not letting it be dynamic I think is pretty sensible 👍

}

@action queryContainer(element) {
this.measureDimensions(element);
this.evaluateQueries();
Expand Down
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@
"@ember/render-modifiers": "^1.0.2",
"ember-cli-babel": "^7.21.0",
"ember-cli-htmlbars": "^5.2.0",
"ember-did-resize-modifier": "^1.0.0"
"ember-did-resize-modifier": "^1.0.0",
"ember-element-helper": "^0.3.1"
},
"devDependencies": {
"@ember/optional-features": "^1.3.0",
Expand Down Expand Up @@ -91,8 +92,8 @@
"ember-template-lint": "^2.9.0",
"ember-test-selectors": "^4.1.0",
"ember-try": "^1.4.0",
"eslint": "^7.3.1",
"eslint-plugin-ember": "^8.9.0",
"eslint": "^7.4.0",
"eslint-plugin-ember": "^8.9.1",
"eslint-plugin-node": "^11.1.0",
"lerna-changelog": "^1.0.1",
"loader.js": "^4.7.0",
Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/components/widgets/widget-1/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
square=(cq-aspect-ratio min=0.8 max=1.25)
wide=(cq-aspect-ratio min=1.25)
}}
@tagName="section"
local-class="container"
>
<header local-class="header">
Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/components/widgets/widget-2/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
tall=(cq-height min=200 max=480)
very-tall=(cq-height min=480)
}}
@tagName="section"
local-class="container"
as |CQ|
>
Expand Down
4 changes: 2 additions & 2 deletions tests/dummy/app/components/widgets/widget-3/index.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div local-class="container">
<section local-class="container">
<header local-class="header">
<h2>Widget 3</h2>

Expand All @@ -17,4 +17,4 @@
@concert={{this.concertData}}
/>
</div>
</div>
</section>
4 changes: 2 additions & 2 deletions tests/dummy/app/components/widgets/widget-4/index.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div local-class="container">
<section local-class="container">
<header local-class="header">
<h2>Widget 4</h2>
</header>
Expand All @@ -12,4 +12,4 @@
All memos
</a>
</div>
</div>
</section>
1 change: 1 addition & 0 deletions tests/dummy/app/components/widgets/widget-4/memo/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
large=(cq-width min=200)
short=(cq-height max=200)
}}
@tagName="article"
local-class="container"
as |CQ|
>
Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/components/widgets/widget-5/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
large=(cq-width min=224)
tall=(cq-height min=120)
}}
@tagName="section"
local-class="container"
as |CQ|
>
Expand Down
6 changes: 6 additions & 0 deletions tests/dummy/config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ module.exports = function(environment) {
}
};

ENV['ember-a11y-testing'] = {
componentOptions: {
turnAuditOff: true
Copy link
Owner Author

Choose a reason for hiding this comment

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

I turned off ember-a11y-testing in development because I ran into an error while testing window resize:

element-resize-detector.js:165 Uncaught Axe is already running. Use `await axe.run()` to wait for the previous run to finish before starting a new run.

}
}

if (environment === 'development') {
// ENV.APP.LOG_RESOLVER = true;
// ENV.APP.LOG_ACTIVE_GENERATION = true;
Expand Down
Loading