-
-
Notifications
You must be signed in to change notification settings - Fork 131
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
Modernize ELT, kill bower and enable yarn #438
Conversation
Apparently, advanced markdown (code fencing, links, etc) does not work yuidoc docblocks for properties and methods
204abff
to
580e0a5
Compare
I accidentally forgot to apply these changes in 533f6df
5aac47c
to
3d47c1c
Compare
Revert this, when babel#5862 is resolved. babel/babel#5862
a9154f2
to
3219c06
Compare
@@ -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. |
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.
technically we only need font-awesome
(e.g. via a CDN), do we care to highlight the difference?
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.
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?
addon/mixins/table-header.js
Outdated
* 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>` | ||
* |
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.
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 ''
*/
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.
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 👍
Thanks for reviewing! 😃 |
Adressing #430.
☠️
bower.json
ember-cli
to 2.13, reconfigure to use npm &ember-source
instead of bowerFaker
/Pretender
shouldn't be needed in bower.json as those are brought in byember-cli-mirage
https://github.com/offirgolan/ember-light-table/blob/master/bower.jsonfont-awesome
should be adevDependency
when we move it to thepackage.json
bower.json
Also
yarn
jquery@2
frombower.json
bootstrap@3.3.7
jshint
toeslint
font-awesome
usage, so it's no surprise that the ascending and descending icons don't work when users drag and drop our code snippets, if they don't havefont-awesome
, and haven't copied our setup.ember-suave
with the spiritual successoreslint-plugin-ember-suave
Likely out of scope
targets
orincludePolyfill
, so that Demo doesn't work in IE11 #427 gets fixed. Was already broken prior to this PR.