This repository has been archived by the owner on Aug 29, 2023. It is now read-only.
feat(tabs): refactors tabs to function closer to spec #1944
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
ajoslin
added
the
in progress
robertmesserle
force-pushed
the
wip-tabs-rewrite
branch
2 times, most recently
from
a9a20bb
to
1b005ff
Compare
robertmesserle
force-pushed
the
wip-tabs-rewrite
branch
10 times, most recently
from
06e27bb
to
12a4ba9
Compare
| angular.module('material.components.tabs') | ||
| .directive('mdTabScroll', MdTabScroll); | ||
|
|
||
| function MdTabScroll () { |
There was a problem hiding this comment.
Can you add comments why this is needed ?
There was a problem hiding this comment.
There's no angular directive for mousewheel to my knowledge. I wrote this to be a generic re-usable mousewheel directive. I named it mdTabScroll for now, but it could easily be reused in other components.
There was a problem hiding this comment.
The implementation seems weird (creating a new scope every time an event happens etc).
Not ot mention sub-optimal from a performance perspective.
Why not follow the paradigm of the rest of the ngEvent directives ?
(Reference: https://github.com/angular/angular.js/blob/master/src/ng/directive/ngEventDirs.js)
BREAKING CHANGE: Generated HTML structure has changed, so custom styles will need to be updated to match the new HTML structure. Closes #1087 Closes #1107 Closes #1140 Closes #1247 Closes #1261 Closes #1380 Closes #1387 Closes #1403 Closes #1443 Closes #1505 Closes #1506 Closes #1516 Closes #1518 Closes #1564 Closes #1570 Closes #1620 Closes #1626 Closes #1698 Closes #1777 Closes #1788 Closes #1850 Closes #1959 Closes #1986 Closes #2020
robertmesserle
force-pushed
the
wip-tabs-rewrite
branch
from
12a4ba9
to
d3c7dce
Compare
ajoslin
removed
in progress
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #1087
Closes #1107
Closes #1140
Closes #1247
Closes #1261
Closes #1380
Closes #1387
Closes #1403
Closes #1443
Closes #1505
Closes #1506
Closes #1516
Closes #1518
Closes #1564
Closes #1570
Closes #1620
Closes #1626
Closes #1698
Closes #1777
Closes #1788
Closes #1850
Closes #1959
Closes #1986