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

stop removing original id of the input #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

velislavbg
Copy link

Current code always removes id of the input when input loses focus (detach). Desired behaviour is to remove id only when a "runtime" id is set on attach-ing :

         if (current_element_random_id_set) {
            current_element[0].removeAttribute('id');
          }

Removing ID happens always because checking whether element has id should be reciprocal to validation when assigning "operational" id of the input i.e.to check element[0] , not element.

changes on lines 149 and 150

@hakib
Copy link
Owner

hakib commented Oct 20, 2017

Hey @velislavbg,

You are right - this is a mistake.

The attributes aria-labelledby and aria-activedescendant are using the generated id's to point to the targeted element and currently selected suggestio. Can you verify that when there is an ID on the target field they are set correctly? I suspect that aria-labelledby might suffer from the same issue (as id is now set on the underlying object and not on an "angular" element).

Thanks for the PR!

@velislavbg
Copy link
Author

You are right : aria-labelledby has a wrong value when we have predefined id, i have changed the code in my fork

I have not used so highly customized suggestion list to come upon a problem with aria-activedescendant.

@hakib
Copy link
Owner

hakib commented Nov 5, 2017

OK, LGTM.

If you could squash the two commits i'll merge and pushlish.

Thanks!

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.

2 participants