idiomorph icon indicating copy to clipboard operation
idiomorph copied to clipboard

Use getAttribute when soft matching ids.

Open dombesz opened this issue 9 months ago • 2 comments

Some javascript frameworks like angular can overwrite the tag's id getter, and comparison can turn into a false negative. Newly added elements don't have the framework's code initialised yet, and calling id on those elements can differ from the id assigned via angular. Since there is no way to have frameworks initialised on the new elements, calling the id can be inconsistent. Using getAttribute("id") ensures that we always compare the html id attributes.

See this issue for more details: #130

dombesz avatar Mar 11 '25 15:03 dombesz

Hey @dombesz , thanks for the PR! A couple of notes before I review:

  1. Don't modify the files in dist/, we rebuild these only just before a release.
  2. Can you get CI green? You can do a simulation locally with npm test:ci. More details in TESTING.md

botandrose avatar Mar 11 '25 17:03 botandrose

Hi @botandrose , thanks for the quick feedback, the CI should be green now.

dombesz avatar Mar 11 '25 21:03 dombesz

Hi @botandrose can you please have a look at this PR?

dombesz avatar May 12 '25 20:05 dombesz

@dombesz Hello! Thank you for your time and effort here, and sorry for the long wait on this!

The problem you pointed out turned out to be more widespread than javascript frameworks like Angular... its an issue with the DOM API itself! Take a look at #135 and #136 for more details. I expect #136 will resolve this as well, do you agree? Can you test the branch in your app to verify?

I'm going to run this branch in production on my app for the next week, and if all looks well, I'll push a bugfix release next sunday.

botandrose avatar Jul 20 '25 12:07 botandrose

closing in favor of #136

botandrose avatar Jul 27 '25 15:07 botandrose

Hi @botandrose , yes I believe the issue is solved by your PR as well. Thanks!

dombesz avatar Jul 27 '25 15:07 dombesz

@dombesz great! I'm glad to hear that.

FYI, I've found another unrelated bug that I want to get into the next bugfix release, so contrary to what I wrote last Sunday, I expect to cut a bugfix release NEXT Sunday now.

botandrose avatar Jul 27 '25 15:07 botandrose