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.

Sign up for GitHub

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/custom icon components #489

Merged
merged 11 commits into from
Oct 26, 2017
Merged

Feat/custom icon components #489

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
35536ea
feat(custom-icons): base files
RedTn Oct 9, 2017
2f4f1ea
fix(column-base): sort icon displayed with sortable:false
RedTn Oct 9, 2017
105c33f
feat(dummy): cookbook for custom-sort-icon
RedTn Oct 9, 2017
0a8ef44
loosen ember-one-way-controls version requirement
bgentry Oct 8, 2017
494f4d0
yarn.lock updates
bgentry Oct 8, 2017
daa657e
[BUGFIX release]: Added check for target column is droppable (#496)
akshay-kr Oct 18, 2017
a8e93a1
[chore]: Update ember-truth-helpers to latest version. (#497)
stopfstedt Oct 18, 2017
2e2faf9
[BUGFIX release]: Ensure the drop target remains properly identified …
RustyToms Oct 18, 2017
a149f96
Merge branch 'feat/custom-icon-components' of https://github.com/RedT…
Oct 23, 2017
7975553
fix(feedback): iconComponent comments, iconComponent test
RedTn Oct 24, 2017
d89ae12
fix(feedback-v2): iconComponent comments, iconComponent test
Oct 24, 2017
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
15 changes: 13 additions & 2 deletions addon/mixins/table-header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ export default Mixin.create({
*/
iconDescending: '',

/**
* See `iconSortable, iconAsending, or iconDescending. Custom sorting component
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change the order

Custom sorting component name to use instead of the default `<i class="lt-sort-icon"></i>` template. 
See `iconSortable`, `iconAsending`, or `iconDescending` for more info.

Uses the same properties.

It's unclear what you mean by this sure what you mean by this

* name to use instead of the default `<i class="lt-sort-icon"></i>` template. Uses
* the same properties.
*
* @property iconComponent
* @type {String}
* @default false
*/
iconComponent: null,

/**
* ID of main table component. Used to generate divs for ember-wormhole
* @type {String}
Expand All @@ -117,8 +128,8 @@ export default Mixin.create({
subColumns: computed.readOnly('table.visibleSubColumns'),
columns: computed.readOnly('table.visibleColumns'),

sortIcons: computed('iconSortable', 'iconAscending', 'iconDescending', function() {
return this.getProperties(['iconSortable', 'iconAscending', 'iconDescending']);
sortIcons: computed('iconSortable', 'iconAscending', 'iconDescending', 'iconComponent', function() {
return this.getProperties(['iconSortable', 'iconAscending', 'iconDescending', 'iconComponent']);
}).readOnly(),

init() {
Expand Down
4 changes: 3 additions & 1 deletion addon/templates/components/columns/base.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
{{component column.component column=column table=table tableActions=tableActions extra=extra sortIcons=sortIcons}}
{{else}}
{{label}}
{{#if sortIconProperty}}
{{#if (and sortIcons.iconComponent sortIconProperty)}}
Copy link
Collaborator

@buschtoens buschtoens Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Can't remember, if we talked about that. but this way the iconComponent would only get rendered when there is a sortIconProperty, i.e. when either of sortable or sorted is true.

I don't have strong feelings either way, since I can't come up with a scenario where you would want to render an iconComponent when you don't have any icons to display. Just wanted to have mentioned 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 remember I ran into this issue in the demo where the avatar column was displaying the sort component when sortable: 'false'.
Because the iconComponent is being specified in the hbs file on the table, all the columns will utilize the iconComponent, so we need a check if the user doesn't want a column to be sortable.

Copy link
Collaborator

@buschtoens buschtoens Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yup, but some users might argue that they'd want their iconComponent to display a "not sortable" icon and that it's up to the user to handle the "not sortable" state instead of ELT doing that for the user.

For instance:

{{#if sortIconProperty}}
  <i class="lt-sort-icon material-icons" style="height:0px;">{{get sortIcons sortIconProperty}}</i>
{{else}}
  <i class="lt-sort-icon material-icons" style="height:0px; color: red;">not_sortable</i>
{{/if}}

Copy link
Contributor Author

@RedTn RedTn Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Understood, I see the use case now. Because you have no strong feelings I guess I'll leave the base.hbs as is for now but feel free to let me know otherwise.

{{component sortIcons.iconComponent sortIcons=sortIcons sortIconProperty=sortIconProperty}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

would you mind adding a test to verify the arguments (sortIcons=sortIcons sortIconProperty=sortIconProperty) get passed to the component declared by iconComponent when we render a table like you do in the demo app:

+  {{t.head
+    onColumnClick=(action 'onColumnClick')
+    iconSortable='unfold_more'
+    iconAscending='keyboard_arrow_up'
+    iconDescending='keyboard_arrow_down'
+    iconComponent='materialize-icon'
+    fixed=true
+  }}

You can steal some ideas from
https://github.com/cibernox/ember-power-select/blob/master/tests/integration/components/power-select/customization-with-components-test.js#L257..L281

{{else if sortIconProperty}}
<i class="lt-sort-icon {{get sortIcons sortIconProperty}}"></i>
{{/if}}
{{/if}}
Expand Down
34 changes: 34 additions & 0 deletions tests/dummy/app/components/cookbook/custom-sort-icon-table.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// BEGIN-SNIPPET custom-sort-icon-table
import Component from '@ember/component';
import TableCommon from '../../mixins/table-common';
import { computed } from '@ember/object';

export default Component.extend(TableCommon, {
columns: computed(function() {
return [{
label: 'Avatar',
valuePath: 'avatar',
width: '60px',
sortable: false,
cellComponent: 'user-avatar'
}, {
label: 'First Name',
valuePath: 'firstName',
width: '150px'
}, {
label: 'Last Name',
valuePath: 'lastName',
width: '150px'
}, {
label: 'Address',
valuePath: 'address'
}, {
label: 'State',
valuePath: 'state'
}, {
label: 'Country',
valuePath: 'country'
}];
})
});
// END-SNIPPET
9 changes: 9 additions & 0 deletions tests/dummy/app/components/materialize-icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// BEGIN-SNIPPET materialize-icon
import Component from '@ember/component';
import layout from '../templates/components/materialize-icon';

export default Component.extend({
tagName: 'span',
layout
});
// END-SNIPPET
1 change: 1 addition & 0 deletions tests/dummy/app/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u" crossorigin="anonymous">
<link href='https://fonts.googleapis.com/css?family=Roboto:400,700,500,300,100|Open+Sans:400,300,600,700' rel='stylesheet' type='text/css'>
<link rel="stylesheet" href="{{rootURL}}assets/dummy.css">
<link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons">

{{content-for "head-footer"}}
</head>
Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Router.map(function() {
this.route('custom-row');
this.route('table-actions');
this.route('horizontal-scrolling');
this.route('custom-sort-icon');
});
});

Expand Down
1 change: 1 addition & 0 deletions tests/dummy/app/routes/cookbook/custom-sort-icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from '../table-route';
3 changes: 3 additions & 0 deletions tests/dummy/app/templates/application.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
{{#link-to 'cookbook.pagination' tagName="li"}}
{{link-to 'Pagination' 'cookbook.pagination'}}
{{/link-to}}
{{#link-to 'cookbook.custom-sort-icon' tagName="li"}}
{{link-to 'Custom Sort Icon' 'cookbook.custom-sort-icon'}}
{{/link-to}}
</ul>
{{/link-to}}
<li><a href="docs">Documentation</a></li>
Expand Down
26 changes: 26 additions & 0 deletions tests/dummy/app/templates/components/cookbook/custom-sort-icon-table.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{{!-- BEGIN-SNIPPET custom-sort-icon-table --}}
{{#light-table table height='65vh' as |t|}}

{{t.head
onColumnClick=(action 'onColumnClick')
iconSortable='unfold_more'
iconAscending='keyboard_arrow_up'
iconDescending='keyboard_arrow_down'
iconComponent='materialize-icon'
fixed=true
}}

{{#t.body
canSelect=false
onScrolledToBottom=(action 'onScrolledToBottom')
as |body|
}}
{{#if isLoading}}
{{#body.loader}}
{{table-loader}}
{{/body.loader}}
{{/if}}
{{/t.body}}

{{/light-table}}
{{!-- END-SNIPPET --}}
3 changes: 3 additions & 0 deletions tests/dummy/app/templates/components/materialize-icon.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{{!-- BEGIN-SNIPPET materialize-icon --}}
<i class="lt-sort-icon material-icons" style="height:0px;">{{get sortIcons sortIconProperty}}</i>
{{!-- END-SNIPPET --}}
12 changes: 12 additions & 0 deletions tests/dummy/app/templates/cookbook/custom-sort-icon.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{{#code-panel
title="Custom Sort Icon"
snippets=(array
"custom-sort-icon-table.js"
"table-common.js"
"custom-sort-icon-table.hbs"
"user-avatar.hbs"
"table-loader.hbs"
"materialize-icon.hbs"
)}}
{{cookbook/custom-sort-icon-table model=model}}
{{/code-panel}}