Conversation
|
hmm, I'm not sure wether this is the best solution. I think adding the wrapper broke not only title, but also the other feature (notifications, etc) that were based on observing the page. Since the page is now really the first child of the page wrapper, won't these be broken? If these features are dead code that no one is using we should delete that code. |
|
seems like 2 possible paths:
I think there are a few things I'd like to change about this module, and I'm sure you've got a list you've been collecting. Ah I see in other PR you agree and we should collect. I probably need to think about this more and read what you've done with patchless more closely to try and understand the IDEA. I've got a bunch of opinions that I've been developing through building scuttle-poll and similar which I'd like to merge with your thinking sometime |
|
I enough things depend on this that I want to be really careful about merging, especially since we don't have tests. if we had tests I'd be way more inclined to merge, because the tests would have caught things such as the titles. this is complicated enough that it needs tests, but currently in the uncanny valley where it works, but without tests. |
Type: Patch
Problem: I previously introduced a bug into hypertabs where I added another level of wrapping to pages when they were added. This then had the MutationObserver watching for
titlechanges on the wrapper instead of the content.Solultion: add a MutationObserver which targets the right thing