Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create div.suggestions-wrapper #28

Merged
merged 1 commit into from
Mar 21, 2019
Merged

create div.suggestions-wrapper #28

merged 1 commit into from
Mar 21, 2019

Conversation

katydecorah
Copy link
Collaborator

fixes #27

This PR adds div.suggestions-wrapper around ul.suggestions to extend customizations to the dropdown.

@katydecorah katydecorah requested a review from tristen March 21, 2019 15:27
Copy link
Owner

@tristen tristen left a comment

Choose a reason for hiding this comment

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

Looks great! One thing: these two functions should probably target wrapper now:

suggestions/src/list.js

Lines 22 to 28 in 262b813

List.prototype.show = function() {
this.element.style.display = 'block';
};
List.prototype.hide = function() {
this.element.style.display = 'none';
};

@katydecorah
Copy link
Collaborator Author

these two functions should probably target wrapper now

I didn't target wrapper on those two functions because if a query doesn't have results then, in our use case, we want div.suggestions-wrapper to continue to display so the user can still submit feedback. (The feedback link is appended to div.suggestions-wrapper.)

Do you think this implementation is too tightly coupled to our use case or do you think it would work in other implementations that you've seen?

@tristen
Copy link
Owner

tristen commented Mar 21, 2019

Ahh gotcha!

Do you think this implementation is too tightly coupled to our use case or do you think it would work in other implementations that you've seen?

Nahhh I think this works 👍

Copy link
Owner

@tristen tristen left a comment

Choose a reason for hiding this comment

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

⚜️

@katydecorah katydecorah merged commit 4fe487a into tristen:gh-pages Mar 21, 2019
@katydecorah katydecorah deleted the patch-1 branch March 21, 2019 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add wrapper div to ul.suggestions to expand customization
2 participants