fix(new-webui): Remove duplicate anchor element from log viewer link.#993
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LogViewerLink
participant AntDesignTheme
User ->> LogViewerLink: Render with props (filePath, logEventIdx, streamId)
LogViewerLink ->> AntDesignTheme: Call theme.useToken()
AntDesignTheme -->> LogViewerLink: Return theme tokens (including colorLink)
LogViewerLink ->> User: Render link with style colour from theme token
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx(2 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>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
🔇 Additional comments (3)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx (3)
5-8: Skip import adjustments. The import oftheme,Tooltip, andTypographyfromantdaligns with the component’s usage and is correctly tree-shaken.
33-34: Verify Ant Design version compatibility fortheme.useToken(). Thetheme.useToken()hook is available only in Ant Design v5+. Please ensure the project dependency is on v5 or later to avoid runtime errors.
39-41:Details
❌ Incorrect review comment
Ensure correct
Linkimport source. Verify thatLinkis imported from"react-router-dom"rather than"react-router", or update the import path to match the routing library in use.
🏁 Script executed:
#!/bin/bash rg "^import.*Link" -n components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsxLength of output: 225
No change needed for
Linkimport
TheLinkcomponent is correctly imported from"react-router"in line with our React Router v7.4.1 setup. You can safely ignore this suggestion.Likely an incorrect or invalid review comment.
| pathname: "/streamFile", | ||
| search: | ||
| `?type=${encodeURIComponent(STREAM_TYPE)}` + | ||
| `&streamId=${encodeURIComponent(streamId)}` + | ||
| `&logEventIdx=${encodeURIComponent(logEventIdx)}`, | ||
| }} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use URLSearchParams for query construction. Refactor the search string to use:
const params = new URLSearchParams({
type: STREAM_TYPE,
streamId,
logEventIdx: String(logEventIdx),
}).toString();This improves readability and maintainability.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
around lines 44 to 49, the query string for the search property is manually
constructed using string concatenation and encodeURIComponent calls. Refactor
this by creating a URLSearchParams instance with an object containing the keys
type, streamId, and logEventIdx (converted to string), then call toString() on
it to generate the query string. Replace the current search string with this
URLSearchParams-generated string to improve readability and maintainability.
| <LinkOutlined/> | ||
| {fileText} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance icon accessibility. Add an aria-label to the <LinkOutlined /> icon (or set aria-hidden="true") to ensure screen readers skip decorative elements.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
at lines 51 to 52, the <LinkOutlined /> icon lacks accessibility attributes. Add
an aria-label describing the icon's purpose or set aria-hidden="true" on the
<LinkOutlined /> component to ensure screen readers skip this decorative
element, improving accessibility.
| target={"_blank"} | ||
| to={{ |
There was a problem hiding this comment.
Mitigate reverse tabnapping risk. Add rel="noopener noreferrer" alongside target="_blank" to prevent potential security vulnerabilities.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
around lines 42 to 43, the anchor element uses target="_blank" without
rel="noopener noreferrer", which exposes a reverse tabnapping security risk. Add
rel="noopener noreferrer" to the anchor or link element alongside
target="_blank" to mitigate this vulnerability.
| <Tooltip title={"Open file"}> | ||
| <Typography.Text> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify JSX attribute. You can replace title={"Open file"} with title="Open file" for cleaner and more idiomatic JSX.
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Message/LogViewerLink.tsx
at lines 37 to 38, simplify the JSX attribute by replacing title={"Open file"}
with title="Open file" to make the code cleaner and more idiomatic.
Description
Browser console was giving error that there were two nested links. This was caused by wrapping reactor router link in antd link for theming purposes. I removed antd link and just passed in the correct blue from the global theme.
Checklist
breaking change.
Validation performed
Error gone.
Summary by CodeRabbit