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

feat(typeahead): Custom CSS class for typeahead's dropdown menu #4410

Closed
wants to merge 4 commits into from

Conversation

gorork
Copy link
Contributor

@gorork gorork commented Sep 13, 2015

PR for #4332 with the corresponding demo.

Typeahead directive checks if an optional attribute typeahead-dropdown-custom-class is specified, and adds it to the dropdown main container. Works with append-to-body attribute.

For example: attribute typeahead-dropdown-custom-class="demo-class" will add demo-class like this:
<div class="dropdown-menu demo-class">.

@wesleycho
Copy link
Contributor

This makes a lot of sense - can you add a test for this feature to verify that the class is properly being set?

@gorork
Copy link
Contributor Author

gorork commented Sep 14, 2015

@wesleycho Added tests for: basic, with a custom template and with append-to-body.

@@ -895,6 +908,12 @@ describe('typeahead tests', function() {
$timeout.flush();
expect(dropdown.css('top')).toEqual('500px');
});

it('should add a custom class to append-to-body popup', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test begs the question, if the user uses typeahead-append-to-body, should the component remove this class from the body tag when the element is destroyed? I think it should here.

@wesleycho
Copy link
Contributor

Left a comment - I think we will need to garbage collect and remove the custom class from the body tag is using append-to-body. Can you add a listener via $scope.$on('$destroy', removeCustomClass) to remove the class anytime the $scope is being destroyed (regardless of append-to-body usage) and add a simple test for this? This should be a quick change, and then we can merge this in.

@gorork
Copy link
Contributor Author

gorork commented Sep 14, 2015

@wesleycho The custom class is being added to a popup element, not to <body> tag. So basically, we get <ul class="dropdown-menu my-class"> element, which can be targeted in CSS, like .dropdown-menu.my-class { ... }.

@wesleycho
Copy link
Contributor

Can you link me to a Plunker with the changes applied so I can see it in action?

@gorork
Copy link
Contributor Author

gorork commented Sep 14, 2015

@wesleycho I've added a demo case for this PR with both typeahead-append-to-body and typeahead-dropdown-custom-class attributes. Would it work instead of Plunkr?

@wesleycho
Copy link
Contributor

Just would be convenient - unfortunately my schedule is super packed for this week and a half, so it could be difficult for me to investigate in detail to verify, and any bit of help would be much appreciated.

@gorork
Copy link
Contributor Author

gorork commented Sep 14, 2015

@wesleycho
Copy link
Contributor

I see, thanks a lot, this looks good to me!

@wesleycho wesleycho closed this in fa1cdfc Sep 15, 2015
jasonaden pushed a commit to deskfed/bootstrap that referenced this pull request Jan 8, 2016
- Adds support for custom classes on the typeahead dropdown

Closes angular-ui#4332
Closes angular-ui#4410
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.

2 participants