Treora opened a new issue #109: URL: https://github.com/apache/incubator-annotator/issues/109
I suggest we reverse this change: https://github.com/apache/incubator-annotator/pull/88 > Define the DOM match functions in terms of Range and place the burden on > the caller to pass a Range. Remove the scope helper functions. > > This change simplifies the code at the possible cost of developer > experience, but understanding the tradeoff will require some experience > and feedback. In this trade-off, I think we should prioritise experience of our users (i.e. developers using our modules). Especially as we’d first still have to ‘acquire’ our users, we may not get much feedback, but simply lose potential users. This is a nice guideline: https://www.w3.org/TR/html-design-principles/#priority-of-constituencies > Priority of Constituencies > > In case of conflict, consider users over authors over implementors over specifiers over theoretical purity. ” I think we should stop insisting on requiring a Range as scopes and targets, for two reasons: - Common cases should be easy. E.g. for matching a textQuoteSelector on a whole page, we now need this: ``` const scope = document.createRange(); scope.selectNodeContents(document); const matches = createTextQuoteSelectorMatcher(textQuoteSelector)(scope); ``` - While for text selectors a range makes sense, for e.g. CssSelector an element is a more logical unit to represent the target. I think that the purpose of using Ranges everywhere was to make refinement easier, and to easily swap between different selector types. But I think we can also achieve this by simply accepting either a Range or a Node/Element for every function, and converting one to the other as needed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
