fix(new-webui): Improve visibility of case-sensitivity toggle (resolves #1058). #1064
Conversation
WalkthroughThe Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
junhaoliao
left a comment
There was a problem hiding this comment.
let's try to fill the validation section in the future to speed up reviews
inactive:
inactive during a search:
active:
active during a search:
i suggest that when isCaseSensitive is inactive but the button is disable (during an in progress query), we can set the button variant to "text" to reduce confusions:
let buttonColor: ButtonColorType = "default";
let buttonVariant: ButtonVariantType = "outlined";
if (isCaseSensitive) {
buttonColor = "primary";
buttonVariant = "solid";
} else if (disabled) {
buttonVariant = "text";
}
otherwise, i think the PR is good to go
since @kirkrodrigues created the issue - do you have any more comments?
|
okay i made ur change, but left the colour |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx (1)
1-71: File-name typo:CaseSenstiveToggle.tsx→CaseSensitiveToggle.tsx.The missing “i” in Senstive will make imports brittle and hurt discoverability. Please rename the file (and update any import paths) before this ships.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: gibber9809
PR: y-scope/clp#504
File: components/core/src/clp_s/search/kql/CMakeLists.txt:29-29
Timestamp: 2024-10-22T15:36:04.655Z
Learning: When reviewing pull requests, focus on the changes within the PR and avoid commenting on issues outside the scope of the PR.
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx (2)
Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.
Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
🔇 Additional comments (2)
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx (2)
33-38: Follow the project guideline: usefalse == exprinstead of logical-NOT.The coding guidelines for
*.tsxfiles specify preferringfalse == <expression>to!<expression>.- onCaseSensitiveChange(!isCaseSensitive); + onCaseSensitiveChange(false == isCaseSensitive);
5-6: UnusedButtonPropsimport, unless you rely on side-effects.
ButtonPropsis pulled in only for thetype ButtonVariantType = …alias below. If you drop that alias (see next comment), the import becomes superfluous and can be deleted to keep the import block lean.⛔ Skipped due to learnings
Learnt from: junhaoliao PR: y-scope/clp#937 File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24 Timestamp: 2025-05-29T20:33:40.653Z Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.Learnt from: davemarco PR: y-scope/clp#797 File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5 Timestamp: 2025-04-08T22:32:05.366Z Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.
| let buttonVariant: ButtonVariantType = "outlined"; | ||
| if (isCaseSensitive) { | ||
| buttonVariant = "solid"; | ||
| } else if (disabled) { | ||
| // The "solid" and "outlined" variants look the same when disabled, | ||
| // so we use "text" variant for a different appearance. | ||
| buttonVariant = "text"; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Edge-case: disabled & case-sensitive results in ambiguous styling.
When disabled is true and isCaseSensitive is true, the variant stays "solid". According to the comment, "solid" and "outlined" look identical when disabled, defeating the intent of highlighting the active state. Consider explicitly handling the disabled && isCaseSensitive branch (e.g., force "outlined" or apply a distinct colour token).
🤖 Prompt for AI Agents
In
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
around lines 40 to 48, the current logic sets buttonVariant to "solid" when
isCaseSensitive is true, even if disabled is also true, causing ambiguous
styling. Update the condition to explicitly handle the case when both disabled
and isCaseSensitive are true by assigning a distinct variant such as "outlined"
or applying a unique color token to differentiate the button's appearance in
this edge case.
| type ButtonVariantType = ButtonProps["variant"]; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inline the type alias and drop the extra level of indirection.
type ButtonVariantType = ButtonProps["variant"] is only referenced once. You can simplify to:
- let buttonVariant: ButtonVariantType = "outlined";
+ let buttonVariant: ButtonProps["variant"] = "outlined";This removes an otherwise-unused alias and the associated import.
🤖 Prompt for AI Agents
In
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
at lines 16-17, the type alias ButtonVariantType is defined but only used once.
Remove the alias and replace its usage directly with ButtonProps["variant"],
eliminating the extra indirection and the import if it becomes unused.
| variant={buttonVariant} | ||
| color={isCaseSensitive ? | ||
| "primary" : | ||
| "default"} | ||
| onClick={handleButtonClick} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
color / variant props are not standard in Ant Design ≥5.
Ant Design’s Button exposes type, ghost, danger, etc., but not variant or color out of the box. If this project wraps antd with custom typings, ignore this warning; otherwise the code will not compile.
🏁 Script executed:
#!/bin/bash
# Do we have a local wrapper/component that augments antd.Button with `variant`/`color`?
rg -t tsx -N "variant={" | headLength of output: 115
I want to confirm whether we’re using a wrapped Button and which antd version we have:
🏁 Script executed:
#!/bin/bash
set -e
echo "1) Inspect antd version in package.json"
grep -R '"antd"' -n package.json || true
echo
echo "2) Detailed from package.json via jq (if installed)"
jq '{deps: .dependencies["antd"], peer: .peerDependencies["antd"], dev: .devDependencies["antd"]}' package.json || true
echo
echo "3) Look for any custom Button.tsx wrapper"
fd -e tsx Button.tsx || true
echo
echo "4) Scan for export or definition of a custom Button component"
rg -n "export .*Button" -t tsx || true
rg -n "const Button" -t tsx || true
echo
echo "5) Find all usages of 'variant=' and 'color=' props in TSX files"
rg -n "variant\s*=" -t tsx || true
rg -n "color\s*=" -t tsx || trueLength of output: 1451
I need to confirm the Button import source and locate the project’s package.json files:
🏁 Script executed:
#!/bin/bash
set -e
echo "1) Show imports and Button usage in CaseSenstiveToggle.tsx"
sed -n '1,50p' components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
echo
echo "2) List all package.json in the repo"
fd -t f package.jsonLength of output: 1696
Replace non-standard variant/color props with Ant Design’s type prop
Ant Design’s ButtonProps (v5+) doesn’t include variant or color. You’re importing directly from "antd", so these props will cause a TS error. Please update the toggle to use type (and other built-in props) instead:
• File: components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
- Remove the
ButtonVariantTypealias (it refers to a non-existentButtonProps["variant"]). - Map your variants to AntD types, for example:
•"solid"→type="primary"
•"outlined"→type="default"(ordashedif preferred)
•"text"→type="text" - Drop the
colorprop altogether.
Example diff:
- import { Button, ButtonProps, Tooltip } from "antd";
+ import { Button, ButtonProps, Tooltip } from "antd";
- type ButtonVariantType = ButtonProps["variant"];
+ // Use built-in ButtonProps["type"] instead of a custom variant
- let buttonVariant: ButtonVariantType = "outlined";
- if (isCaseSensitive) {
- buttonVariant = "solid";
- } else if (disabled) {
- buttonVariant = "text";
- }
+ const buttonType: ButtonProps["type"] =
+ isCaseSensitive ? "primary" :
+ disabled ? "text" :
+ "default";
return (
<Tooltip …>
<Button
- variant={buttonVariant}
- color={isCaseSensitive ? "primary" : "default"}
+ type={buttonType}
onClick={handleButtonClick}
disabled={disabled}
>
…📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variant={buttonVariant} | |
| color={isCaseSensitive ? | |
| "primary" : | |
| "default"} | |
| onClick={handleButtonClick} | |
| import { Button, ButtonProps, Tooltip } from "antd"; | |
| // Use built-in ButtonProps["type"] instead of a custom variant | |
| const buttonType: ButtonProps["type"] = | |
| isCaseSensitive ? "primary" : | |
| disabled ? "text" : | |
| "default"; | |
| return ( | |
| <Tooltip …> | |
| <Button | |
| type={buttonType} | |
| onClick={handleButtonClick} | |
| disabled={disabled} | |
| > | |
| … | |
| </Button> | |
| </Tooltip> | |
| ); |
🤖 Prompt for AI Agents
In
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
around lines 56 to 60, the Button component incorrectly uses non-standard props
`variant` and `color` which are not supported by Ant Design v5+. Remove the
`ButtonVariantType` alias and replace the `variant` and `color` props with the
Ant Design `type` prop, mapping your existing variants to appropriate `type`
values such as "primary" for solid and "default" or "dashed" for outlined. Also,
remove the `color` prop entirely to fix the TypeScript errors.
junhaoliao
left a comment
There was a problem hiding this comment.
let's wait for @kirkrodrigues to take a look before merging this
kirkrodrigues
left a comment
There was a problem hiding this comment.
Approving appearance. Deferring to @junhaoliao's review for everything else.
Description
Changes toggle styling per issue #1058
Checklist
breaking change.
Validation performed
Summary by CodeRabbit