search

phetsims / utterance-queue

Alerting library powered by aria-live
MIT License
0 stars 2 forks source link

utterance-queue is dependent on scenery #33

Open jessegreenberg opened 3 years ago

jessegreenberg commented 3 years ago

utterance-queue currently has a dependency on scenery because of a usage of PDOMUtils in AriaHerald. But scenery depends on utterance-queue, so this introduces a circular dependency. How should we handle? Move PDOMUtils to something that is a dependency for both scenery and utterance-queue? Move just the used function? Duplicate the used function?

jessegreenberg commented 3 years ago

I think PDOMUtils has too many things that are scenery specific or unique to PDOM to move the whole file out of scenery. I recommend we make a new file called DOMUtils.js in phet-core that will have setTextContent and other things that are useful for DOM operations but not unique to PDOM related work.

@zepumph does this seem OK to you? If so, reassign and I am happy to do this.

zepumph commented 3 years ago

Yes, I think that is a really good idea. Thanks!

jessegreenberg commented 3 years ago

I tried to do this but setTextContent uses validate which lives in axon. I don't think we should introduce axon as a dependency of phet-core because axon already depends on phet-core and that would trade this circular dependency for another.

jessegreenberg commented 3 years ago

I wondered about moving validate out of axon because it seems useful for more than just observables and inquired about that in https://github.com/phetsims/axon/issues/361. I think this is on hold until we get a yay or nay over there.

jessegreenberg commented 3 years ago

While discussing https://github.com/phetsims/axon/issues/361, @samreid suggested that the validate portion of setTextContent could just be removed so that we could move it into phet-core. We can copy the regex in ValidatorDef.STRING_WITHOUT_TEMPLATE_VARS_VALIDATOR, or move IT into phet-core to be used in scenery.

jessegreenberg commented 2 years ago

I tried this and noticed that a number of things would have to move from PDOMUtils to DOMUtils to support this (trimLeft, INPUT_TAG, tagNameSupportsContent, ELEMENTS_WITHOUT_CLOSING_TAG, containsFormattingTags).

Actually the majority of PDOMUtils is not specific to PDOM and could make sense in a DOMUtils. I think going through the following steps to move most of PDOMUtils into phet-core would preserve history best. 1) Rename PDOMUtils to DOMUtils. 2) Move DOMUtils to phet-core using copy-history-to-different-repo.sh 3) Re-create PDOMUtils in scenery and move portions of DOMUtils that are specific to the PDOM PDOMUtils.

Would that work and make sense? Alternatively, both PDOMUtils and DOMUtils will exist at the end of this and a commit message like "move many PDOMUtils functions into phet-core so utterance-queue doesn't have to depend on scenery" will link history between the two.

I am happy to do either but don't want to mess up the history in this move! @zepumph which would you recommend? I ask since you have knowledge of copy-history-to-different-repo and have expressed an interest in preserving history in the past.

zepumph commented 2 years ago

I like this idea! I thought that it would be best to move to phet-core, but it depends on axon, so utterance-queue sounds good for now.

zepumph commented 2 years ago

From meeting with JG today, we will rename to DOMUtils and move to utterance-queue using the copy history script.