Copilot commented on code in PR #5756:
URL: https://github.com/apache/texera/pull/5756#discussion_r3432106274
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.ts:
##########
@@ -258,20 +258,33 @@ export class AdminUserComponent implements OnInit {
this.listOfDisplayUser = [...this.userList];
}
+ private normalizeSearchValue(value: string | null | undefined): string {
+ return (value ?? "").trim().toLowerCase();
+ }
+
searchByName(): void {
this.nameSearchVisible = false;
+ this.emailSearchValue = "";
+ this.commentSearchValue = "";
const q = (this.nameSearchValue ?? "").trim().toLowerCase();
- this.listOfDisplayUser = this.userList.filter(u => (u.name ??
"").toLowerCase().includes(q));
+ this.listOfDisplayUser = this.userList.filter(user => (user.name ??
"").toLowerCase().includes(q));
Review Comment:
`searchByName` trims for comparison but leaves `nameSearchValue` unchanged,
so a whitespace-only search still shows the filter as active
(`nameSearchValue.length > 0` in the template). Trim and store the value before
filtering, and use a lowercased copy for the includes-check.
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.ts:
##########
@@ -258,20 +258,33 @@ export class AdminUserComponent implements OnInit {
this.listOfDisplayUser = [...this.userList];
}
+ private normalizeSearchValue(value: string | null | undefined): string {
+ return (value ?? "").trim().toLowerCase();
+ }
+
searchByName(): void {
this.nameSearchVisible = false;
+ this.emailSearchValue = "";
+ this.commentSearchValue = "";
const q = (this.nameSearchValue ?? "").trim().toLowerCase();
- this.listOfDisplayUser = this.userList.filter(u => (u.name ??
"").toLowerCase().includes(q));
+ this.listOfDisplayUser = this.userList.filter(user => (user.name ??
"").toLowerCase().includes(q));
}
searchByEmail(): void {
this.emailSearchVisible = false;
- this.listOfDisplayUser = this.userList.filter(user => (user.email ||
"").indexOf(this.emailSearchValue) !== -1);
+ this.nameSearchValue = "";
+ this.commentSearchValue = "";
+
+ const q = (this.emailSearchValue ?? "").trim().toLowerCase();
+ this.listOfDisplayUser = this.userList.filter(user => (user.email ??
"").toLowerCase().includes(q));
}
searchByComment(): void {
this.commentSearchVisible = false;
- this.listOfDisplayUser = this.userList.filter(user => (user.comment ||
"").indexOf(this.commentSearchValue) !== -1);
+ this.nameSearchValue = "";
+ this.emailSearchValue = "";
+ const q = (this.commentSearchValue ?? "").trim().toLowerCase();
+ this.listOfDisplayUser = this.userList.filter(user => (user.comment ??
"").toLowerCase().includes(q));
}
Review Comment:
Similar to other searches: trim and store `commentSearchValue` before
filtering so whitespace-only input doesn’t leave the Comment filter marked
active, while still doing a case-insensitive comparison.
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.ts:
##########
@@ -258,20 +258,33 @@ export class AdminUserComponent implements OnInit {
this.listOfDisplayUser = [...this.userList];
}
+ private normalizeSearchValue(value: string | null | undefined): string {
+ return (value ?? "").trim().toLowerCase();
+ }
+
searchByName(): void {
this.nameSearchVisible = false;
+ this.emailSearchValue = "";
+ this.commentSearchValue = "";
const q = (this.nameSearchValue ?? "").trim().toLowerCase();
- this.listOfDisplayUser = this.userList.filter(u => (u.name ??
"").toLowerCase().includes(q));
+ this.listOfDisplayUser = this.userList.filter(user => (user.name ??
"").toLowerCase().includes(q));
}
searchByEmail(): void {
this.emailSearchVisible = false;
- this.listOfDisplayUser = this.userList.filter(user => (user.email ||
"").indexOf(this.emailSearchValue) !== -1);
+ this.nameSearchValue = "";
+ this.commentSearchValue = "";
+
+ const q = (this.emailSearchValue ?? "").trim().toLowerCase();
+ this.listOfDisplayUser = this.userList.filter(user => (user.email ??
"").toLowerCase().includes(q));
Review Comment:
Similar to name search: trim and store `emailSearchValue` before filtering
so whitespace-only input doesn’t leave the Email filter marked active, while
still doing a case-insensitive comparison.
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.ts:
##########
@@ -258,20 +258,33 @@ export class AdminUserComponent implements OnInit {
this.listOfDisplayUser = [...this.userList];
}
+ private normalizeSearchValue(value: string | null | undefined): string {
+ return (value ?? "").trim().toLowerCase();
+ }
+
searchByName(): void {
this.nameSearchVisible = false;
+ this.emailSearchValue = "";
+ this.commentSearchValue = "";
const q = (this.nameSearchValue ?? "").trim().toLowerCase();
- this.listOfDisplayUser = this.userList.filter(u => (u.name ??
"").toLowerCase().includes(q));
+ this.listOfDisplayUser = this.userList.filter(user => (user.name ??
"").toLowerCase().includes(q));
}
searchByEmail(): void {
this.emailSearchVisible = false;
- this.listOfDisplayUser = this.userList.filter(user => (user.email ||
"").indexOf(this.emailSearchValue) !== -1);
+ this.nameSearchValue = "";
+ this.commentSearchValue = "";
+
+ const q = (this.emailSearchValue ?? "").trim().toLowerCase();
Review Comment:
The search normalization/clearing behavior is user-facing and easy to
regress (case-insensitive email/comment matching, trimming, and clearing
inactive search fields). Please add focused unit tests in
`admin-user.component.spec.ts` to cover these cases (can test the component
methods directly without DOM interaction).
##########
frontend/src/app/dashboard/component/admin/user/admin-user.component.ts:
##########
@@ -258,20 +258,33 @@ export class AdminUserComponent implements OnInit {
this.listOfDisplayUser = [...this.userList];
}
+ private normalizeSearchValue(value: string | null | undefined): string {
+ return (value ?? "").trim().toLowerCase();
+ }
Review Comment:
`normalizeSearchValue` is currently unused; if you start using it to
normalize the bound input values, returning a lowercased string will also
lowercase what the user sees in the filter input. Consider making this helper
only trim whitespace, and apply `.toLowerCase()` only to the comparison value.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
