Merge "gr-comment: Add "." to text property for gr-suggestion-textarea"
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 8ca09f9..6a9f35b 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1956,6 +1956,55 @@
If some branches could not be deleted, the response is "`409 Conflict`" and the
error message is contained in the response body.
+[[delete-changes]]
+=== Delete Changes
+--
+'POST /projects/link:#project-name[\{project-name\}]/changes:delete'
+--
+
+Delete one or more changes.
+
+The changes to be deleted must be provided in the request body as a
+link:#delete-changes-input[DeleteChangesInput] entity.
+
+.Request
+----
+ POST /projects/MyProject/changes:delete HTTP/1.0
+ Content-Type: application/json;charset=UTF-8
+
+ {
+ "changes": [
+ "changeId1",
+ "changeId2",
+ "changeId3",
+ "changeId4"
+ ]
+ }
+----
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "FAILURE": [
+ "changeId1"
+ ],
+ "NOT_UNIQUE": [
+ "changeId2"
+ ].
+ "SUCCESS": [
+ "changeId3",
+ "changeId4"
+ ]
+ }
+----
+
+If some changes could not be deleted, the response will be "`200 OK`" with the details in the response body.
+
+
[[get-content]]
=== Get Content
--
diff --git a/java/com/google/gerrit/extensions/api/projects/DeleteChangesInput.java b/java/com/google/gerrit/extensions/api/projects/DeleteChangesInput.java
new file mode 100644
index 0000000..2c90926
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/projects/DeleteChangesInput.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.projects;
+
+import java.util.List;
+
+public class DeleteChangesInput {
+ public List<String> changes;
+}
diff --git a/java/com/google/gerrit/extensions/api/projects/DeleteChangesResult.java b/java/com/google/gerrit/extensions/api/projects/DeleteChangesResult.java
new file mode 100644
index 0000000..c70d828
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/projects/DeleteChangesResult.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.projects;
+
+public enum DeleteChangesResult {
+ NOT_UNIQUE,
+ SUCCESS,
+ FAILURE
+}
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index a59cbd5..f96755a 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -72,6 +72,9 @@
void deleteBranches(DeleteBranchesInput in) throws RestApiException;
+ Map<DeleteChangesResult, Collection<String>> deleteChanges(DeleteChangesInput in)
+ throws RestApiException;
+
void deleteTags(DeleteTagsInput in) throws RestApiException;
abstract class ListRefsRequest<T extends RefInfo> {
diff --git a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 44882ba..ea9d09e 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -36,6 +36,8 @@
import com.google.gerrit.extensions.api.projects.DashboardApi;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
+import com.google.gerrit.extensions.api.projects.DeleteChangesInput;
+import com.google.gerrit.extensions.api.projects.DeleteChangesResult;
import com.google.gerrit.extensions.api.projects.DeleteTagsInput;
import com.google.gerrit.extensions.api.projects.DescriptionInput;
import com.google.gerrit.extensions.api.projects.HeadInput;
@@ -73,6 +75,7 @@
import com.google.gerrit.server.restapi.project.CreateAccessChange;
import com.google.gerrit.server.restapi.project.CreateProject;
import com.google.gerrit.server.restapi.project.DeleteBranches;
+import com.google.gerrit.server.restapi.project.DeleteChanges;
import com.google.gerrit.server.restapi.project.DeleteTags;
import com.google.gerrit.server.restapi.project.GetAccess;
import com.google.gerrit.server.restapi.project.GetConfig;
@@ -137,6 +140,7 @@
private final Provider<ListBranches> listBranches;
private final Provider<ListTags> listTags;
private final DeleteBranches deleteBranches;
+ private final DeleteChanges deleteChanges;
private final DeleteTags deleteTags;
private final CommitsCollection commitsCollection;
private final CommitApiImpl.Factory commitApi;
@@ -183,6 +187,7 @@
Provider<ListBranches> listBranches,
Provider<ListTags> listTags,
DeleteBranches deleteBranches,
+ DeleteChanges deleteChanges,
DeleteTags deleteTags,
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
@@ -227,6 +232,7 @@
listBranches,
listTags,
deleteBranches,
+ deleteChanges,
deleteTags,
project,
commitsCollection,
@@ -275,6 +281,7 @@
Provider<ListBranches> listBranches,
Provider<ListTags> listTags,
DeleteBranches deleteBranches,
+ DeleteChanges deleteChanges,
DeleteTags deleteTags,
CommitsCollection commitsCollection,
CommitApiImpl.Factory commitApi,
@@ -319,6 +326,7 @@
listBranches,
listTags,
deleteBranches,
+ deleteChanges,
deleteTags,
null,
commitsCollection,
@@ -366,6 +374,7 @@
Provider<ListBranches> listBranches,
Provider<ListTags> listTags,
DeleteBranches deleteBranches,
+ DeleteChanges deleteChanges,
DeleteTags deleteTags,
ProjectResource project,
CommitsCollection commitsCollection,
@@ -410,6 +419,7 @@
this.listBranches = listBranches;
this.listTags = listTags;
this.deleteBranches = deleteBranches;
+ this.deleteChanges = deleteChanges;
this.deleteTags = deleteTags;
this.commitsCollection = commitsCollection;
this.commitApi = commitApi;
@@ -656,6 +666,16 @@
}
@Override
+ public Map<DeleteChangesResult, Collection<String>> deleteChanges(DeleteChangesInput in)
+ throws RestApiException {
+ try {
+ return deleteChanges.apply(project, in).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot delete changes", e);
+ }
+ }
+
+ @Override
public void deleteTags(DeleteTagsInput in) throws RestApiException {
try {
@SuppressWarnings("unused")
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index 6e6fb82..3cc578d 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -221,16 +221,29 @@
try (Timer0.Context ctx = metrics.filterVisibility.start()) {
for (Account.Id reviewer : sortedRecommendations) {
if (filteredRecommendations.size() >= limit) {
+ logger.atFine().log("Skip results because the limit (%s) has been reached.", limit);
break;
}
if (suggestReviewers.isSkipServiceUsers()
&& serviceUserClassifier.isServiceUser(reviewer)) {
+ logger.atFine().log("Filter out %s because it's a service user", reviewer);
continue;
}
// Check if change is visible to reviewer and if the current user can see reviewer
- if (visibilityControl.isVisibleTo(reviewer) && accountControl.canSee(reviewer)) {
- filteredRecommendations.add(reviewer);
+ if (!visibilityControl.isVisibleTo(reviewer)) {
+ logger.atFine().log("Filter out %s because this user cannot see the change", reviewer);
+ continue;
}
+
+ // Check if the current user can see reviewer
+ if (!accountControl.canSee(reviewer)) {
+ logger.atFine().log(
+ "Filter out %s because the caller (%s) cannot see the account",
+ reviewer, self.get().getLoggableName());
+ continue;
+ }
+
+ filteredRecommendations.add(reviewer);
}
}
logger.atFine().log("Filtered recommendations: %s", filteredRecommendations);
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteChanges.java b/java/com/google/gerrit/server/restapi/project/DeleteChanges.java
new file mode 100644
index 0000000..9868354
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/DeleteChanges.java
@@ -0,0 +1,116 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.FAILURE;
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.NOT_UNIQUE;
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.SUCCESS;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.api.projects.DeleteChangesInput;
+import com.google.gerrit.extensions.api.projects.DeleteChangesResult;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.change.ChangeFinder;
+import com.google.gerrit.server.change.DeleteChangeOp;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.inject.Inject;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+public class DeleteChanges implements RestModifyView<ProjectResource, DeleteChangesInput> {
+
+ private final PermissionBackend permissionBackend;
+ private final ChangeFinder changeFinder;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final DeleteChangeOp.Factory opFactory;
+
+ @Inject
+ public DeleteChanges(
+ PermissionBackend permissionBackend,
+ ChangeFinder changeFinder,
+ BatchUpdate.Factory batchUpdateFactory,
+ DeleteChangeOp.Factory opFactory) {
+ this.permissionBackend = permissionBackend;
+ this.changeFinder = changeFinder;
+ this.batchUpdateFactory = batchUpdateFactory;
+ this.opFactory = opFactory;
+ }
+
+ @Override
+ public Response<Map<DeleteChangesResult, Collection<String>>> apply(
+ ProjectResource resource, DeleteChangesInput input)
+ throws RestApiException, UpdateException, PermissionBackendException {
+ if (input == null || input.changes == null || input.changes.isEmpty()) {
+ throw new BadRequestException("Change Ids must be specified");
+ }
+ ListMultimap<DeleteChangesResult, String> responseBody = ArrayListMultimap.create();
+ List<String> deletableChanges = new ArrayList<>();
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ try (BatchUpdate bu =
+ batchUpdateFactory.create(resource.getNameKey(), resource.getUser(), TimeUtil.now())) {
+ for (String change : input.changes) {
+ List<ChangeNotes> cn = changeFinder.find(change);
+ if (cn.isEmpty()) {
+ responseBody.put(FAILURE, change);
+ } else if (cn.size() > 1) {
+ responseBody.put(NOT_UNIQUE, change);
+ } else {
+ if (isChangeDeletable(cn.getFirst())) {
+ checkPermissions(cn);
+ Change.Id changeId = cn.getFirst().getChange().getId();
+ bu.addOp(changeId, opFactory.create(changeId));
+ deletableChanges.add(change);
+ } else {
+ responseBody.put(FAILURE, change);
+ }
+ }
+ }
+ bu.execute();
+ responseBody.putAll(SUCCESS, deletableChanges);
+ }
+ }
+ return Response.ok(responseBody.asMap());
+ }
+
+ public void checkPermissions(List<ChangeNotes> cn)
+ throws PermissionBackendException, AuthException {
+ if (cn.getFirst() != null) {
+ permissionBackend.currentUser().change(cn.getFirst()).check(ChangePermission.DELETE);
+ }
+ }
+
+ private static boolean isChangeDeletable(ChangeNotes cn) {
+ // Merged changes must not be deleted.
+ // New or abandoned changes can be deleted with the right permissions.
+ return !cn.getChange().isMerged();
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
index 18dbe13..f5647ec 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
@@ -128,6 +128,8 @@
delete(TAG_KIND).to(DeleteTag.class);
post(PROJECT_KIND, "tags:delete").to(DeleteTags.class);
+
+ post(PROJECT_KIND, "changes:delete").to(DeleteChanges.class);
}
/** Separately bind batch functionality. */
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteChangesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteChangesIT.java
new file mode 100644
index 0000000..d0a3012
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteChangesIT.java
@@ -0,0 +1,93 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.FAILURE;
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.NOT_UNIQUE;
+import static com.google.gerrit.extensions.api.projects.DeleteChangesResult.SUCCESS;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
+import com.google.gerrit.extensions.api.projects.DeleteChangesInput;
+import com.google.gerrit.extensions.api.projects.DeleteChangesResult;
+import com.google.gerrit.extensions.api.projects.ProjectApi;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+@NoHttpd
+public class DeleteChangesIT extends AbstractDaemonTest {
+
+ private ProjectApi project() throws Exception {
+ return gApi.projects().name(project.get());
+ }
+
+ @Test
+ public void deleteChangesFailure() throws Exception {
+ DeleteChangesInput deleteInput = new DeleteChangesInput();
+ deleteInput.changes = List.of("Non-existing-change-1", "Non-existing-change-2");
+
+ Map<DeleteChangesResult, Collection<String>> response = project().deleteChanges(deleteInput);
+ assertThat(response).containsKey(FAILURE);
+ assertThat(response.get(FAILURE))
+ .containsExactly("Non-existing-change-1", "Non-existing-change-2");
+ }
+
+ @Test
+ public void deleteChangesSuccess() throws Exception {
+ PushOneCommit.Result c1 = createChange();
+ PushOneCommit.Result c2 = createChange();
+ assertThat(c1.getChange().change().getProject().get()).isNotNull();
+ assertThat(c2.getChange().change().getProject().get()).isNotNull();
+ DeleteChangesInput deleteInput = new DeleteChangesInput();
+ deleteInput.changes = List.of(c1.getChangeId(), c2.getChangeId());
+ Map<DeleteChangesResult, Collection<String>> response = project().deleteChanges(deleteInput);
+ assertThat(response).containsKey(SUCCESS);
+ assertThat(response.get(SUCCESS)).containsExactly(c1.getChangeId(), c2.getChangeId());
+ assertThat(query(c1.getChangeId())).isEmpty();
+ assertThat(query(c2.getChangeId())).isEmpty();
+ }
+
+ @Test
+ public void deleteChangesSuccessFailureNotUnique() throws Exception {
+ TestRepository<InMemoryRepository> repo = cloneProject(project);
+ PushOneCommit.Result c1 = createChange(repo, "master", "Add a file", "foo", "content", null);
+ PushOneCommit.Result c2 = createChange();
+
+ // cherry pick a change to make a duplicate change for NOT_UNIQUE case.
+ String newBranch = "Test-branch";
+ createBranch(BranchNameKey.create(project, newBranch));
+ CherryPickInput cpi = new CherryPickInput();
+ cpi.destination = newBranch;
+ gApi.changes().id(c1.getChangeId()).current().cherryPick(cpi);
+
+ DeleteChangesInput deleteInput = new DeleteChangesInput();
+ deleteInput.changes = List.of(c1.getChangeId(), c2.getChangeId(), "Non-existing-change-1");
+ Map<DeleteChangesResult, Collection<String>> response = project().deleteChanges(deleteInput);
+ assertThat(response).containsKey(FAILURE);
+ assertThat(response.get(FAILURE)).containsExactly("Non-existing-change-1");
+ assertThat(response).containsKey(SUCCESS);
+ assertThat(response.get(SUCCESS)).containsExactly(c2.getChangeId());
+ assertThat(response).containsKey(NOT_UNIQUE);
+ assertThat(response.get(NOT_UNIQUE)).containsExactly(c1.getChangeId());
+ }
+}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index e064399..b3f96d4 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -874,7 +874,15 @@
+ "Contains-Conflicts: true\n"
+ "Ours: 2d1a400a2e56090699f8aeb522ec1f82bbd54d57\n"
+ "Theirs: aaeceb9f08df45748b1420ab2b0687906151ae59\n");
- assertThat(changeNotesState.patchSets().getLast().getValue().conflicts().get().noBaseReason())
+ assertThat(
+ changeNotesState.patchSets().stream()
+ .filter(e -> e.getKey().get() == 2)
+ .findFirst()
+ .get()
+ .getValue()
+ .conflicts()
+ .get()
+ .noBaseReason())
.hasValue(NoMergeBaseReason.HISTORIC_DATA_WITHOUT_BASE);
// Base/Ours/Theirs/Merge-strategy/No-base-reason is ignored if Contains-Conflicts is missing
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
index 7c4db7f..ffa0ce4 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts
@@ -124,6 +124,7 @@
--md-menu-container-color: var(--dropdown-background-color);
--md-menu-top-space: 0px;
--md-menu-bottom-space: 0px;
+ --md-focus-ring-duration: 0s;
}
md-divider {
margin: auto;
@@ -233,7 +234,6 @@
this.cursor.setCursorAtIndex(this.selectedIndex);
if (this.cursor.target !== null) {
this.cursor.target.focus();
- this.handleAddSelected();
}
this.setUpGlobalEventListeners();
} else {
@@ -294,6 +294,7 @@
tabindex="-1"
.menuCorner=${'start-start'}
?quick=${true}
+ .skipRestoreFocus=${true}
@click=${this.handleDropdownClick}
@opened=${(e: Event) => {
this.opened = true;
@@ -303,6 +304,7 @@
this.opened = false;
// This is an ugly hack but works.
this.cursor.target?.removeAttribute('selected');
+ this.cursor.target?.blur();
}}
>
${incrementalRepeat({
@@ -399,18 +401,14 @@
* Handle the up key.
*/
private handleUp() {
- this.handleRemoveSelected();
this.cursor.previous();
- this.handleAddSelected();
}
/**
* Handle the down key.
*/
private handleDown() {
- this.handleRemoveSelected();
this.cursor.next();
- this.handleAddSelected();
}
/**
@@ -420,7 +418,6 @@
if (this.cursor.target !== null) {
const el = this.cursor.target.shadowRoot?.querySelector(':not([hidden])');
if (el) {
- this.handleRemoveSelected();
(el as HTMLElement).click();
}
}
@@ -487,30 +484,6 @@
);
}
}
-
- private handleRemoveSelected() {
- // We workaround an issue to allow cursor to work.
- // For some reason without this, it doesn't work half the time.
- // E.g. you press enter or you close the dropdown, reopen it,
- // you expect it to be focused with the first item selected.
- // The below fixes it. It's an ugly hack but works for now.
- const mdFocusRing = this.cursor.target?.shadowRoot
- ?.querySelector('md-item')
- ?.querySelector('md-focus-ring');
- if (mdFocusRing) mdFocusRing.visible = false;
- }
-
- private handleAddSelected() {
- // We workaround an issue to allow cursor to work.
- // For some reason without this, it doesn't work half the time.
- // E.g. you press enter or you close the dropdown, reopen it,
- // you expect it to be focused with the first item selected.
- // The below fixes it. It's an ugly hack but works for now.
- const mdFocusRing = this.cursor.target?.shadowRoot
- ?.querySelector('md-item')
- ?.querySelector('md-focus-ring');
- if (mdFocusRing) mdFocusRing.visible = true;
- }
}
declare global {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
index a7c84df..a8b62cc 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.ts
@@ -72,6 +72,7 @@
--md-menu-container-color: var(--dropdown-background-color);
--md-menu-top-space: 0px;
--md-menu-bottom-space: 0px;
+ --md-focus-ring-duration: 0s;
}
gr-button {
vertical-align: top;
@@ -206,7 +207,6 @@
this.cursor.setCursorAtIndex(0);
if (this.cursor.target !== null) {
this.cursor.target.focus();
- this.handleAddSelected();
}
this.setUpGlobalEventListeners();
} else {
@@ -264,6 +264,7 @@
: 'end-start'}
.yOffset=${this.verticalOffset}
?quick=${true}
+ .skipRestoreFocus=${true}
@opened=${() => {
this.opened = true;
}}
@@ -271,6 +272,7 @@
this.opened = false;
// This is an ugly hack but works.
this.cursor.target?.removeAttribute('selected');
+ this.cursor.target?.blur();
}}
>
${this.renderDropdownContent()}
@@ -377,18 +379,14 @@
* Handle the up key.
*/
private handleUp() {
- this.handleRemoveSelected();
this.cursor.previous();
- this.handleAddSelected();
}
/**
* Handle the down key.
*/
private handleDown() {
- this.handleRemoveSelected();
this.cursor.next();
- this.handleAddSelected();
}
/**
@@ -400,7 +398,6 @@
if (this.cursor.target !== null) {
const el = this.cursor.target.shadowRoot?.querySelector(':not([hidden])');
if (el) {
- this.handleRemoveSelected();
(el as HTMLElement).click();
}
}
@@ -492,7 +489,8 @@
if (e.currentTarget === null || !this.items) {
return;
}
- const id = (e.currentTarget as Element).getAttribute('data-id');
+ const target = e.currentTarget as HTMLElement;
+ const id = target.getAttribute('data-id');
const item = this.items.find(item => item.id === id);
if (id && !this.disabledIds.includes(id)) {
if (item) {
@@ -513,28 +511,4 @@
);
}
}
-
- private handleRemoveSelected() {
- // We workaround an issue to allow cursor to work.
- // For some reason without this, it doesn't work half the time.
- // E.g. you press enter or you close the dropdown, reopen it,
- // you expect it to be focused with the first item selected.
- // The below fixes it. It's an ugly hack but works for now.
- const mdFocusRing = this.cursor.target?.shadowRoot
- ?.querySelector('md-item')
- ?.querySelector('md-focus-ring');
- if (mdFocusRing) mdFocusRing.visible = false;
- }
-
- private handleAddSelected() {
- // We workaround an issue to allow cursor to work.
- // For some reason without this, it doesn't work half the time.
- // E.g. you press enter or you close the dropdown, reopen it,
- // you expect it to be focused with the first item selected.
- // The below fixes it. It's an ugly hack but works for now.
- const mdFocusRing = this.cursor.target?.shadowRoot
- ?.querySelector('md-item')
- ?.querySelector('md-focus-ring');
- if (mdFocusRing) mdFocusRing.visible = true;
- }
}
