Skip to content

refactor: use JS this instead of passing the element via $0 in Element.executeJs #24601

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.

Sign up for GitHub

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

Open
Artur- wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: use JS this instead of passing the element via $0 in Element.executeJs #24601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions flow-dnd/src/main/java/com/vaadin/flow/component/dnd/DragSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,9 @@ default void setDragImage(Component dragImage, int offsetX, int offsetY) {
ComponentUtil.setData(getDragSourceComponent(),
DndUtil.DRAG_SOURCE_IMAGE, dragImage);
getDraggableElement().executeJs(
"window.Vaadin.Flow.dndConnector.setDragImage($0, $1, $2, $3)",
"window.Vaadin.Flow.dndConnector.setDragImage($0, $1, $2, this)",
dragImage, (dragImage == null ? 0 : offsetX),
(dragImage == null ? 0 : offsetY), getDraggableElement());
(dragImage == null ? 0 : offsetY));
}

private void appendDragElement(Element dragElement) {
Expand Down
6 changes: 2 additions & 4 deletions flow-dnd/src/main/java/com/vaadin/flow/component/dnd/internal/DndUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ private DndUtil() {
public static <T extends Component> void updateDragSourceActivation(
DragSource<T> dragSource) {
Command command = () -> dragSource.getDraggableElement().executeJs(
"window.Vaadin.Flow.dndConnector.updateDragSource($0)",
dragSource.getDraggableElement());
"window.Vaadin.Flow.dndConnector.updateDragSource(this)");
runOnAttachBeforeResponse(dragSource.getDragSourceComponent(), command);
}

Expand All @@ -128,8 +127,7 @@ public static <T extends Component> void updateDragSourceActivation(
public static <T extends Component> void updateDropTargetActivation(
DropTarget<T> dropTarget) {
Command command = () -> dropTarget.getElement().executeJs(
"window.Vaadin.Flow.dndConnector.updateDropTarget($0)",
dropTarget.getElement());
"window.Vaadin.Flow.dndConnector.updateDropTarget(this)");

runOnAttachBeforeResponse(dropTarget.getDropTargetComponent(), command);

Expand Down
36 changes: 18 additions & 18 deletions flow-server/src/main/java/com/vaadin/flow/component/Focusable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,27 +139,27 @@ default void focus(FocusOption... options) {
if (json == null) {
// No options, call focus() without arguments
element.executeJs("""
setTimeout(function(){
setTimeout(() => {
try {
$0._nextFocusIsFromClient = false;
$0.focus();
this._nextFocusIsFromClient = false;
this.focus();
} finally {
$0._nextFocusIsFromClient = true;
this._nextFocusIsFromClient = true;
}
},0)
""", element);
}, 0)
""");
} else {
// Call focus with options object passed as parameter
element.executeJs("""
setTimeout(function(){
setTimeout(() => {
try {
$0._nextFocusIsFromClient = false;
$0.focus($1);
this._nextFocusIsFromClient = false;
this.focus($0);
} finally {
$0._nextFocusIsFromClient = true;
this._nextFocusIsFromClient = true;
}
},0)
""", element, json);
}, 0)
""", json);
}
}

Expand Down Expand Up @@ -190,15 +190,15 @@ default void focus() {
*/
default void blur() {
getElement().executeJs("""
setTimeout(function(){
setTimeout(() => {
try {
$0._nextBlurIsFromClient = false;
$0.blur();
this._nextBlurIsFromClient = false;
this.blur();
} finally {
$0._nextBlurIsFromClient = true;
this._nextBlurIsFromClient = true;
}
},0)
""", getElement());
}, 0)
""");
}

/**
Expand Down
5 changes: 2 additions & 3 deletions flow-server/src/main/java/com/vaadin/flow/dom/Element.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2148,10 +2148,9 @@ public Element scrollIntoView(ScrollIntoViewOption... options) {

// Use setTimeout to work on newly created elements
if (json == null) {
executeJs("setTimeout(function(){$0.scrollIntoView()},0)", this);
executeJs("setTimeout(() => this.scrollIntoView(), 0)");
} else {
executeJs("setTimeout(function(){$0.scrollIntoView($1)},0)", this,
json);
executeJs("setTimeout(() => this.scrollIntoView($0), 0)", json);
}

return getSelf();
Expand Down
6 changes: 3 additions & 3 deletions flow-server/src/test/java/com/vaadin/flow/component/ComponentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2071,12 +2071,12 @@ private void assertScrollIntoViewWithParams(String... expectedJsonParts) {
// Verify it uses parameter passing
String expression = inv.getExpression();
MatcherAssert.assertThat(expression,
CoreMatchers.containsString("$0.scrollIntoView($1)"));
CoreMatchers.containsString("this.scrollIntoView($0)"));

// Verify parameters contain expected JSON parts
List<Object> params = inv.getParameters();
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
for (String expectedPart : expectedJsonParts) {
MatcherAssert.assertThat(paramJson,
CoreMatchers.containsString(expectedPart));
Expand Down
54 changes: 27 additions & 27 deletions flow-server/src/test/java/com/vaadin/flow/component/FocusableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ void focus_withFocusVisible_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"focusVisible\":true"),
"Should set focusVisible to true");
assertFalse(paramJson.contains("preventScroll"),
Expand All @@ -123,15 +123,15 @@ void focus_withFocusNotVisible_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"focusVisible\":false"),
"Should set focusVisible to false");
}
Expand All @@ -148,15 +148,15 @@ void focus_withPreventScrollEnabled_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"preventScroll\":true"),
"Should set preventScroll to true");
assertFalse(paramJson.contains("focusVisible"),
Expand All @@ -175,15 +175,15 @@ void focus_withPreventScrollDisabled_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"preventScroll\":false"),
"Should set preventScroll to false");
}
Expand All @@ -200,15 +200,15 @@ void focus_withBothOptions_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"preventScroll\":true"),
"Should set preventScroll to true");
assertTrue(paramJson.contains("\"focusVisible\":true"),
Expand All @@ -227,15 +227,15 @@ void focus_withBothOptionsFalse_generatesCorrectJS() {
String expression = invocations.get(0).getInvocation().getExpression();
assertTrue(expression.contains("setTimeout"),
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus($1)"),
assertTrue(expression.contains(".focus($0)"),
"Should contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.get(0).getInvocation()
.getParameters();
// First param is element, second param is the options object
assertTrue(params.size() >= 2, "Should have at least 2 parameters");
String paramJson = params.get(1).toString();
// First param is the options object
assertTrue(params.size() >= 1, "Should have at least 1 parameter");
String paramJson = params.get(0).toString();
assertTrue(paramJson.contains("\"preventScroll\":false"),
"Should set preventScroll to false");
assertTrue(paramJson.contains("\"focusVisible\":false"),
Expand All @@ -257,13 +257,13 @@ void focus_withoutOptions_generatesCorrectJS() {
"Should contain setTimeout wrapper");
assertTrue(expression.contains(".focus()"),
"Should contain focus call without parameters");
assertFalse(expression.contains(".focus($1)"),
assertFalse(expression.contains(".focus($0)"),
"Should not contain focus call with parameter");

// Check the parameters
List<Object> params = invocations.getFirst().getInvocation()
.getParameters();
assertEquals(2, params.size(),
"Should have exactly 1 parameter (the element node and wrapped parameter)");
assertEquals(1, params.size(),
"Should have exactly 1 wrapped parameter (no user-provided parameters)");
}
}
Loading