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

feat(Header & Footer): Add header & footer to the dropdown list. #1930

Merged
merged 2 commits into from
Mar 20, 2017

Conversation

felixmosh
Copy link
Contributor

This feature allows to add header & footer to the dropdowns for each of the default templates.
bootstrap
select2
selectize

@Jefiozie
Copy link
Contributor

Jefiozie commented Mar 7, 2017

HI @felixmosh
Thanks for submitting this PR, looks like it will help resolve #1323 but before I can review this issue could you clean up the PR, you committed the dist directory and that is not correct. Also I believe a rebase is needed for the last changes.

Thanks looking forward on the review.

@felixmosh
Copy link
Contributor Author

Done.

@Jefiozie Jefiozie self-requested a review March 7, 2017 07:17
left: 0px !important;
top: 0px !important;
left: 0 !important;
top: 0 !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason why you changed the 0px to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Convention wise, in css if you have 0px it should be 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the important needed?

Copy link
Contributor Author

@felixmosh felixmosh Mar 9, 2017

Choose a reason for hiding this comment

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

I don`t know it was there before me :]

}

.select2-result-single {
padding-left: 0;
}

.select2-locked > .select2-search-choice-close{
display:none;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you fix all the spacing in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -1,5 +1,4 @@
<div ng-show="$select.open" class="ui-select-choices ui-select-dropdown selectize-dropdown"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why you removed all the classes from this div?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

missed that one.

@@ -1,7 +1,7 @@
'use strict';

describe('ui-select tests', function() {
Copy link
Contributor

@Jefiozie Jefiozie Mar 9, 2017

Choose a reason for hiding this comment

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

Could you fix the spacing in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@Jefiozie Jefiozie left a comment

Choose a reason for hiding this comment

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

See my comments 😄

@Jefiozie
Copy link
Contributor

Jefiozie commented Mar 9, 2017

Second could you have a look at the wiki this is from a PR before. Maybe you can update it when this PR is merged.

@felixmosh
Copy link
Contributor Author

The wiki is up to date to the current implementation :).
I`ve added additional example how to use it in each of the templates.

@ahules
Copy link

ahules commented Mar 13, 2017

How we can use header/footer now? I mean what URL we must add to bower.json?

@Jefiozie
Copy link
Contributor

@ahules Do you mean when this will be release so that you can use it with bower?

@ahules
Copy link

ahules commented Mar 14, 2017

@Jefiozie yes, now I use TechNaturally/ui-select#feat-header-footer-bower. But that realisation of header/footer triggers click twice.
Maybe you can create some branch named "experimental" or smth. :)

@Jefiozie
Copy link
Contributor

This will be released in the newer version of this repo. For now i'm not making any experimental branches. As i need to maintain that branch also and it is hard enough to maintain the repo on it own.
If need you can use the code as committed here.

Hope you understand.

@ahules
Copy link

ahules commented Mar 14, 2017

@Jefiozie, Can you help? I will create fork and merge this commit manually?

@Jefiozie
Copy link
Contributor

I don't have the time to help you having a hard time to maintaining this repo.
I hope to update the lib by the end of this month so then you can use the official library for the functionality.

@Jefiozie
Copy link
Contributor

Hi @felixmosh Could you resolve the conflict? (don't know why this is suddenly here) afte this i will merge the PR. Thanks for your help.

@felixmosh
Copy link
Contributor Author

Done

@Jefiozie Jefiozie merged commit 47883ed into angular-ui:master Mar 20, 2017
@cyrilchapon
Copy link

This breaks 100% width and ul bullet inside ui-select-choices on my application.

See

before (0.19.6)
image

after (0.19.7)
image

@felixmosh
Copy link
Contributor Author

@Jefiozie why this PR was removed?

@Jefiozie
Copy link
Contributor

There where two main reasons.

  1. The angular injection was not correct (I had to address that to you sorry for that)
  2. Second , for some reason the complete layout was corrupt. Still need to have a look why this happend.

@felixmosh
Copy link
Contributor Author

felixmosh commented Apr 23, 2017

@Jefiozie, I've fixed that already, I will create a new PR.
Any other design issues that you know of, except ul bullets and the width?

@felixmosh
Copy link
Contributor Author

Fresh PR #1982

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.

4 participants