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

Feature - header & footer #1323

Closed
wants to merge 1 commit into from

Conversation

plong0
Copy link
Contributor

@plong0 plong0 commented Nov 25, 2015

allows ui-select-header and ui-select-footer directives to be embedded in the select dropdown.

Example Structure with header and footer:

<ui-select>
    <ui-select-match></ui-select-match>
    <ui-select-header>Top of the list!</ui-select-header>
    <ui-select-choices><ui-select-choices>
    <ui-select-footer>Bottom of the list.</ui-select-footer>
</ui-select>

@plong0
Copy link
Contributor Author

plong0 commented Nov 25, 2015

Not sure what's up with travis tests failing, all karma tests pass when I run local gulp. Looks like most recent PR's are failing their travis tests too.

@plong0
Copy link
Contributor Author

plong0 commented Nov 25, 2015

Bonus side effect - using ui-scrollpoint inside a ui-select-footer, I was easily able to add infinite scrolling to my ui-select choices.

@joshrickert
Copy link

The infinite scrolling benefit is pretty cool.

Since you're prepending/appending the header and footer to the ul element, does that mean the header and footer scroll into/out of view? I think the folks over in #496 were hoping for a static overlay above and below the scrolling area.

@plong0 plong0 force-pushed the feat-header-footer branch from 44f43df to e58cd3f Compare November 25, 2015 17:48
@plong0
Copy link
Contributor Author

plong0 commented Nov 25, 2015

Oh yes, you are right. They do scroll into and out of view. I think you could might be able to have them stay in place with css? I will need to test that. If not, I will think about adding something like a fixed="true" attribute.

@joshrickert
Copy link

I suspected a CSS solution might be applicable too. A quick search around Stack Overflow indicates it might be a bit tricky, though.

That said, I'm not sure there is much that can be done without seriously reorganizing the templates since the top-level element of choices.tpl.html is an ul element. I needed a header for my project, so I did an add-on directive to implement the header functionality, and the approach was much the same as you took.

@plong0
Copy link
Contributor Author

plong0 commented Nov 25, 2015

Ok, well I just read this article: http://webdesign.tutsplus.com/tutorials/sticky-positioning-with-nothing-but-css--cms-24042

Looks like it should be as easy as postion: sticky; but as the article notes, it is not supported in all browsers yet :(
I tested position: fixed; in chrome, it worked for the header. I am not going to call it a solution though, because position: fixed; is always supposed to be to the window, not the parent.

I tried using the ngSticky module with no success.

If I get the time, I'll look more into something to make them sticky. I bet there's a way to use ui-scrollpoint for this too.

@joshrickert
Copy link

Would it make sense to transclude the directives a little higher up the DOM tree?

@joshrickert
Copy link

Also, I was trying out your code and thought it looked a bit better if I included a bootstrap list separator as well:

transcludedChoices.prepend(angular.element('<li>').addClass('divider').prop('role', 'presentation'));
transcludedChoices.prepend(transcludedHeader);

@plong0
Copy link
Contributor Author

plong0 commented Jan 26, 2016

Hmm not sure if it would work with the directives higher in the DOM tree. If I recall, it is the <ul> element that gets shown/hidden/scrolled.

@joshrickert
Copy link

Yeah, there would have to be some new CSS and custom logic to show/hide those elements, or maybe just a new container around the ul element that can be toggled instead.

For what it's worth, I have your current code monkey-patched in and it's working well.

Is fixing the travis build just a matter of merging upstream changes?

@plong0
Copy link
Contributor Author

plong0 commented Jan 26, 2016

ya just merged upstream and travis build passes now.

@plong0
Copy link
Contributor Author

plong0 commented Jan 26, 2016

I've got a gulped branch at https://github.com/TechNaturally/ui-select/tree/feat-header-footer-bower

You can use it in your bower.json dependencies with:
"angular-ui-select": "TechNaturally/ui-select#feat-header-footer-bower"

@wesleycho
Copy link
Contributor

The history on this needs fixing - there should not be any merge commit.

@plong0
Copy link
Contributor Author

plong0 commented Mar 29, 2016

Fixed history

@joshrickert
Copy link

It would probably also be good to add documentation and an example so that users know the feature exists once it is available.

@plong0
Copy link
Contributor Author

plong0 commented Apr 1, 2016

Just added a wiki page. Working on an infinite-scroll example using ui-select-footer and ui-scrollpoint

@user378230
Copy link
Contributor

You'll need to rebase especially following changes to examples in #1550 (you can just move your <script> to the body and update demo.js

A basic header footer example would also be nice, rename to demo-header-and-footer.html (include both basic and infinite scrolling on this page)

Once that's done I'll merge.

@plong0 plong0 force-pushed the feat-header-footer branch 2 times, most recently from a396a80 to 58d49b7 Compare April 8, 2016 15:43
@plong0
Copy link
Contributor Author

plong0 commented Apr 8, 2016

@user378230 thanks... rebased, and renamed/updated the demo to include a basic example.

I am not sure that I could use a <script> tag though, because I need to add ui.scrollpoint as a dependency of demo module and add the infinity object onto DemoCtrl's scope vm. If you could please advise a method to do this from a <script>, I would be happy to.

@user378230
Copy link
Contributor

@GDuchemin please post meaningful responses to issues, thanks!

Also not that this PR doesn't work with angular 1.5.x as per previous replies, can't be merged until it does.

@GDuchemin
Copy link

GDuchemin commented Aug 1, 2016

I just tried to reselect the target for prepend and append transcludeHeader and transcludeFooter with element.querySelectorAll('.ui-select-choices'), wrapped all in $timeout and it's seems working.

@user378230
Copy link
Contributor

@GDuchemin maybe post your code?

@GDuchemin
Copy link

GDuchemin commented Aug 1, 2016

uiSelectDirective.js after line 230:

var transcludedHeader = transcluded.querySelectorAll('.ui-select-header');
          if(transcludedHeader && transcludedHeader.length){
            transcludedHeader.removeAttr('ui-select-header'); //To avoid loop in case directive as attr
            transcludedHeader.removeAttr('data-ui-select-header'); // Properly handle HTML5 data-attributes
            $timeout(function(){
              element.querySelectorAll('.ui-select-choices').prepend(transcludedHeader);
            });
          }

         var transcludedFooter = transcluded.querySelectorAll('.ui-select-footer');
          if(transcludedFooter && transcludedFooter.length){
            transcludedFooter.removeAttr('ui-select-footer'); //To avoid loop in case directive as attr
            transcludedFooter.removeAttr('data-ui-select-footer'); // Properly handle HTML5 data-attributes
            $timeout(function() {
              element.querySelectorAll('.ui-select-choices').append(transcludedFooter);
            });
          }

I just defined again the selector before prepend and append code.

@plong0 plong0 force-pushed the feat-header-footer branch 2 times, most recently from 908f374 to 2218ed6 Compare August 1, 2016 20:27
@plong0
Copy link
Contributor Author

plong0 commented Aug 1, 2016

Thanks so much for the input @GDuchemin! your solution for appending after $timeout works in angular 1.4 + 1.5. You brought this back to life.

I've rebased my branch and done a bit more work to refactor with the $timeout. I also added a $compile of the ui-select-choices after appending header and footer.

The reason for the re-compile is in case any directives in the header or footer require the ui-select-choices or any of its attribute directives. For example: in the infinite-scroll demo, the ui-scrollpoint inside the ui-select-footer requires the ui-scrollpoint-target on the ui-select-choices in order to detect when the choices have been scrolled.

Plunker demo updated with latest compiled select.js+select.css copied in.

Everything seems to be working with angular 1.4 + 1.5!

I think this branch might finally be ready to merge...

updated uiSelect directive's transclusion to include ui-select-header and ui-select-footer elements
@plong0 plong0 force-pushed the feat-header-footer branch from 2218ed6 to b743fcc Compare August 1, 2016 20:42
@GDuchemin
Copy link

Great work @plong0 :)

@dannydinges
Copy link

When will this be merged?

@user378230
Copy link
Contributor

@dannydinges if you watch the repo you should get a github notification when this thread is updated, you can then pop by to see if it's been merged. Hope that helps! 😃

@kotmatpockuh
Copy link

kotmatpockuh commented Aug 28, 2016

@plong0, maybe you know, why ng-click in ui-select-header is fired twice?

http://plnkr.co/edit/KzL6bpGDeA5yPReDFhQ9?p=preview

@margielm
Copy link

Hello,
Any idea when this PR will be merged and released?

@plong0
Copy link
Contributor Author

plong0 commented Sep 21, 2016

@user378230 what is the needs: PR test(s) label?

@chansx
Copy link

chansx commented Oct 18, 2016

@kotmatpockuh I also encountered the same problem. Did you solve the problem?

@gpsinghsandhu
Copy link

@kotmatpockuh @chansx The problem seems to be this. Commenting out the $compile call (line 254 in src/uiSelectDirective.js) worked for me as there was no need for an extra compile in my use case.

@plong0
Copy link
Contributor Author

plong0 commented Nov 28, 2016

@kotmatpockuh @chansx @gpsinghsandhu

Based on the answer @gpsinghsandhu linked to, maybe it needs to $compile each transcluded bit separately rather than the entire selectChoices - Something like this

@artgryn
Copy link

artgryn commented Dec 6, 2016

Probably merge and release? =)

@shyamal890
Copy link

Any updates on this?

@felixmosh
Copy link
Contributor

felixmosh commented Feb 28, 2017

@kotmatpockuh, i encountered the same problem, even worse, ng-if will generate each time you open & close the list a new element.

The reason is that compliation: https://github.com/TechNaturally/ui-select/blob/b743fcce965ad6ed1ab4a200e961d119af774b42/src/uiSelectDirective.js#L254

Update: @plong0, your solution to compile header & footer separately didn`t solved the issue, what was the reason of the second compilation?

@felixmosh
Copy link
Contributor

felixmosh commented Mar 4, 2017

I`ve created a clean PR that implements header & footer without hacks.
You can create a theme for it, there is no double compilation therefore there is no double click trigger bugs.
#1982

@igorbar
Copy link

igorbar commented Mar 22, 2017

Could somebody say when this PR will be released?

@Jefiozie
Copy link
Contributor

It has been released. Closing this PR down.

@Jefiozie Jefiozie closed this Apr 12, 2017
@georgeslegros
Copy link

Could you please validate the release number it is supposed to be included in? I checked the latest and

  1. select.js still says version 0.19.7
  2. select.js does not contain any reference to "footer"
  3. the directive uiSelectFooterDirective.js is not in the src folder anymore

Was the feature removed or something?

Thanks

@felixmosh
Copy link
Contributor

It was removed, there is another PR that waits for a long time to be merged.
#1982

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.