-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Move getElementFromSelector
& getSelectorFromElement
#36027
Conversation
d84c041
to
7d5daed
Compare
7d5daed
to
a4b16f3
Compare
edde81f
to
c7721e5
Compare
c7721e5
to
d98e376
Compare
d98e376
to
f3e62e9
Compare
f3e62e9
to
0b54300
Compare
0b54300
to
79ed268
Compare
79ed268
to
3f36e65
Compare
ca34019
to
4f38b46
Compare
4f38b46
to
3739fd7
Compare
3739fd7
to
8847c81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR LGTM but I got a question of architecture.
For the Manipulator
we can do stuff like Manipulator.getDataAttributes()
. We don't want here to be able to do SelectorEngine.getElementFromSelector()
, SelectorEngine.getMultipleElementsFromSelector()
and SelectorEngine.getSelectorFromElement()
?
It would probably allow to export it like export default SelectorEngine
and so not being forced to import it with brackets like we had to do for example in import { SelectorEngine } from '../../../src/dom/selector-engine'
. And would be consistent with all the files in js/src/dom/*.js
that are built like this.
Was thinking of something like that: patch.txt (git apply patch.txt
to see the diff)
Of course, I am ok with this approach. I tried to do the minimum changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change LGTM! I let others double-check that :)
cbddc6f
to
c185ccf
Compare
Move `getElementFromSelector` & getSelectorFromElement` inside selector-engine.js, in order to use SelectorEngine methods, avoiding raw querySelector usage Feature: add `getMultipleElementsFromSelector` helper
c185ccf
to
ce38f11
Compare
Move
getElementFromSelector
&getSelectorFromElement
inside selector-engine.js, in order to use SelectorEngine methods, avoiding rawquerySelector
usageFeature: add
getMultipleElementsFromSelector
helper