Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

chore(refactor): split code into multiple directives #731

Merged
merged 1 commit into from
Mar 12, 2015

Conversation

dimirc
Copy link
Contributor

@dimirc dimirc commented Mar 9, 2015

Recently we started organizing the library on #684 when code was separated into multiple files, but I still see too much conditional code to check if the component is on single/multiple selection mode. So I think that will be better to split the code into 2 different directives, uisSingle and uisMultiple respectively that will internally be appended to the element automatically on main uiSelect compile fn, depending on the presence or absence of multiple attribute.

PD: the public API wont change, its just internal

New files created:

  • uiSelectSingleDirective.js
  • uiSelectMultipleDirective.js

@dimirc dimirc force-pushed the refactor-split-directives branch 3 times, most recently from 2d1c5f3 to 205fae2 Compare March 11, 2015 05:38
@dimirc
Copy link
Contributor Author

dimirc commented Mar 11, 2015

I believe this PR is looking better. Maybe just a couple more changes and will be ready to merge, since I'd like to avoid people to submit new PRs under the current organization on master and then be forced to rebase

@@ -0,0 +1,393 @@
uis.directive('uisMultiple', ['uiSelectMinErr','$timeout', function(uiSelectMinErr, $timeout) {
Copy link

Choose a reason for hiding this comment

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

Is this really the directive name you want?

<uis-multiple>

Aren't you afraid of the confusion it will cause by being in a totally different namespace from the other directives in this same library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prefix uis is already used on some filenames/directives of this library to keep some names shorter, but we could also consider uiSelectMultipleand uiSelectSingle for this new directives if thats cleaner.

dimirc added a commit that referenced this pull request Mar 12, 2015
chore(refactor): split code into multiple directives
@dimirc dimirc merged commit 8fa1e69 into master Mar 12, 2015
@dimirc dimirc deleted the refactor-split-directives branch March 12, 2015 04:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants