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

Solves Append the list to body #41 #491

Closed
wants to merge 3 commits into from
Closed

Solves Append the list to body #41 #491

wants to merge 3 commits into from

Conversation

dhaval-patel
Copy link

Created a layer at body level and appended dropdown to it. So it will not affect the height of actual control and it will also prevent mouse wheel after dropdown is opened.

@dmccown1500
Copy link

If this works well lets get in in!

@dimirc dimirc added this to the 0.11.x milestone Feb 20, 2015
@lebster lebster mentioned this pull request Mar 6, 2015
@leipert
Copy link

leipert commented Mar 6, 2015

@dhaval-patel Could you please add the changes to the corresponding files in the src folder instead of the dist file? We cannot see what you changed.

@amcdnl
Copy link
Contributor

amcdnl commented Mar 6, 2015

Please remove the dist folder and only submit the src changes.

@cmlenz
Copy link
Contributor

cmlenz commented Mar 6, 2015

#718 also implements this feature.

@dhaval-patel
Copy link
Author

@amcdnl done !!

@dhaval-patel
Copy link
Author

@cmlenz My solution has default as "Append to Body". But I can make it as your saying. It will be much better.

@leipert
Copy link

leipert commented Mar 7, 2015

@dhaval-patel . No, please have a look at #718 for example. You should not have changes in the dist folder. The dist should not appear at all!
And the changes you made should be in the individual src files. We do not see what you changed. If you change the uiSelect directive, change uiSelectDirective.js.

If you want to test ui-select with your new feature you have to

$ npm install
$ gulp test
$ gulp build

If you do not have gulp installed globally, do it with npm install -g gulp

@dhaval-patel
Copy link
Author

@leipert I don't see any individual files in my repo. My repo contains only select.js. May be you guys have changed your repo structure after I pulled.

@leipert
Copy link

leipert commented Mar 7, 2015

Ah okay, I did not know. I am not a maintainer ;) Well hopefully one will come along and give some advice and either merge this PR or #718

@brianfeister
Copy link

@dhaval-patel there was a major structural change in v0.9.9 and your CSS indicates that your branched off at 0.9.4. Sorry for the inconvenience but I can't see your changes with the current PR, you need to rebase current master onto your branch before I can review this. Also, I don't think this will be merged because #718 seems to do the same thing and is already properly rebased.

@dimirc dimirc closed this Aug 1, 2015
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.

7 participants