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

Modernize ELT, kill bower and enable yarn #438

Conversation

buschtoens
Copy link
Collaborator

@buschtoens buschtoens commented Jun 21, 2017

Adressing #430.

☠️ bower.json

Also

Likely out of scope

@buschtoens buschtoens force-pushed the 430-modernize-elt-and-kill-bower branch from 204abff to 580e0a5 Compare June 21, 2017 10:20
@buschtoens buschtoens force-pushed the 430-modernize-elt-and-kill-bower branch from 5aac47c to 3d47c1c Compare June 21, 2017 12:04
@buschtoens buschtoens changed the title [WIP] Modernize ELT and kill bower Modernize ELT and kill bower Jun 21, 2017
@buschtoens buschtoens changed the title Modernize ELT and kill bower Modernize ELT, kill bower and enable yarn Jun 21, 2017
@@ -1,6 +1,11 @@
{{!-- BEGIN-SNIPPET horizontal-scrolling-table --}}
{{#light-table table height='65vh' as |t|}}

{{!--
In order for `fa-sort-asc` and `fa-sort-desc` icons to work,
you need to have ember-font-awesome installed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

technically we only need font-awesome (e.g. via a CDN), do we care to highlight the difference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, yeah. Good catch.

  {{!--
    In order for `fa-sort-asc` and `fa-sort-desc` icons to work,
    you need to have ember-font-awesome installed or manually include
    the font-awesome assets, e.g. via a CDN.
  --}}

Would that be better?

* For instance, if you have installed `ember-font-awesome`, you can set
* `iconAscending` to `'fa fa-sort-asc'`, which would yield this markup:
* `<i class="lt-sort-icon fa fa-sort-asc"></i>`
*
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In accordance with #438 (comment) this might be better as well?

  /**
   * CSS classes to be applied to an `<i class="lt-sort-icon></i>` tag that is
   * inserted into the column's `<th>` element.
   *
   * For instance, if you have installed `ember-font-awesome` or include the
   * `font-awesome` assets manually (e.g. via a CDN), you can set
   * `iconAscending` to `'fa fa-sort-asc'`, which would yield this markup:
   * `<i class="lt-sort-icon fa fa-sort-asc"></i>`
   *
   * @property iconAscending
   * @type {String}
   * @default ''
   */

Copy link
Collaborator

Choose a reason for hiding this comment

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

in lieu of having documentation that explains what icon fonts are & how they work (I couldn't find anything on the web), this well explained documentation with an example will do 👍

@buschtoens buschtoens merged commit 867b980 into adopted-ember-addons:master Jun 21, 2017
@buschtoens
Copy link
Collaborator Author

Thanks for reviewing! 😃

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.

2 participants