Changing activeClass, added disabled state, moved to Mixin, ember-cli 2.4.1, and more! #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated the ember-cli dep to 2.4.1 and ran `ember init` to update other deps and file changes.
Removed `ember-data` from the `devDependencies`, not needed for this addon..
Really minor, just removing un-needed `.gitkeep` files since the addon and app folders contain stuff.
Make the `childLinkViews` property an Ember Array, instead of having the Computed Properties convert it (setup for future commits). This commit includes a few other related changes; * Create the initial array in the `init()` hook * Run `_super()` on the `didRender()` hook * Change `var` to `let` for variable declarations
Previously, the active class name was hard coded to ‘active’. Now it
will pull the child `{{link-to}}` `activeClass` name by default. Or,
users can specify what it will be by defining the `activeClass` on the
`{{active-link}}`. Also added tests for this and updated the README
(which has been expanded to show more examples).
As a negative to active, add a disabled state that adds a class when
ALL child links are also disabled. Similarly, the class name draws from
the child `{{link-to}}` but can be overridden. Users can always opt-out
by defining `disabledClass=false`. Also added tests for this and
updated the README.
While probably a rare case, expose the selector used to find child links. That way if folks are using `<button>`s instead, they could update the `linkSelector`. Also added tests for this and updated the README.
By moving the main functionality of this addon into a Mixin, Components from other addons can implement these features. For example, other UI libraries might provide a dropdown Component where this might be a helpful addition.
Just a few tweaks to the README; broke-up long lines, extra spacing between headers, added the Development section (in new addons), adjusted the tests link, and removed the Building section (doesn’t make sense for this addon).
|
@alexspeller Just want to ping ya for feedback. Been a couple weeks now and just didn't want to lose sight of this PR. I'll jump on Slack for a few hours if you'd rather chat. Again, no hurry, just keeping this on my todo list. Edit: Just realized you're probably offline for the day (with the time difference and all). I'll try to be online tomorrow morning at some point. |
| }), | ||
|
|
||
| didRender: function() { | ||
| init() { | ||
| this._super( ...arguments ); |
There was a problem hiding this comment.
There is no need to call super here. init on Ember.Component is an empty function.
|
Really sorry this took so long to merge, it's a great PR and I have just been incredibly ill. Thanks! |
|
version |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finally got around to creating the PR for issue #10. This encompasses everything mentioned there, with your concerns resolved (I'm hoping anyway). Here are the commit messages and details:
Update to ember-cli 2.4.1
Updated the ember-cli dep to 2.4.1 and ran
ember initto update other deps and file changes.Remove ember-data
Removed
ember-datafrom thedevDependencies, not needed for this addon..Remove un-needed .gitkeep files
Really minor, just removing un-needed
.gitkeepfiles since the addon and app folders contain stuff.Make 'childLinkViews' an Ember Array
Make the
childLinkViewsproperty an Ember Array, instead of having the Computed Properties convert it (setup for future commits). This commit includes a few other related changes;init()hook_super()on thedidRender()hookvartoletfor variable declarationsAdd ability to change the 'activeClass' name
Previously, the active class name was hard coded to ‘active’. Now it will pull the child
{{link-to}}activeClassname by default. Or, users can specify what it will be by defining theactiveClasson the{{active-link}}. Also added tests for this and updated the README (which has been expanded to show more examples).Add support for the disabled state
As a negative to active, add a disabled state that adds a class when ALL child links are also disabled. Similarly, the class name draws from the child
{{link-to}}but can be overridden. Users can always opt-out by definingdisabledClass=false. Also added tests for this and updated the README.Expose the child link selector
While probably a rare case, expose the selector used to find child links. That way if folks are using
<button>s instead, they could update thelinkSelector. Also added tests for this and updated the README.Move functionality to mixin
By moving the main functionality of this addon into a Mixin, Components from other addons can implement these features. For example, other UI libraries might provide a dropdown Component where this might be a helpful addition.
Minor README tweaks
Just a few tweaks to the README; broke-up long lines, extra spacing between headers, added the Development section (in new addons), adjusted the tests link, and removed the Building section (doesn’t make sense for this addon).
Impacts on existing issues and PRs:
Resolves #3
Resolves #10
Closes #8