feat: active alerts display values . from issue #2873 - #2236
Conversation
📝 WalkthroughWalkthroughThe alert current-events page adds a persisted switch for showing trigger values. ChangesAlert trigger value display
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Switch
participant AlertCurrentEventsPage
participant localStorage
participant AlertTable
Switch->>AlertCurrentEventsPage: Toggle showTriggerValue
AlertCurrentEventsPage->>localStorage: Store preference
AlertCurrentEventsPage->>AlertTable: Pass showTriggerValue
AlertTable-->>AlertCurrentEventsPage: Render trigger_value column when enabled
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/pages/alertCurEvent/pages/List/AlertTable.tsx (1)
222-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the new column type-safe.
The
as anycast at Line 222 suppresses column and renderer type checking. Use the existing alert-record type with an Ant DesignColumnsTypedefinition, then add the column without the cast.As per coding guidelines: Declare component Props explicitly with TypeScript interface and avoid
any.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx` at line 222, Remove the `as any` cast from the new alert-table column and define the columns using Ant Design’s `ColumnsType` with the existing alert-record type, preserving type checking for the column and renderer. Also declare the component Props through an explicit TypeScript interface and avoid introducing any `any` types.Source: Coding guidelines
src/pages/alertCurEvent/pages/List/index.tsx (1)
350-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Tailwind for the new toolbar container.
The new
Spacecontrols flex and gap layout. Replace it with a small container usingflex items-center gap-2or the existing toolbar utility.As per coding guidelines: Use Tailwind utility classes for container layout (flex, grid, gap, padding, margin, alignment).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/alertCurEvent/pages/List/index.tsx` around lines 350 - 375, Replace the new Space wrapper around the show-trigger Switch and expand Button with a container using Tailwind layout utilities, such as flex items-center gap-2, while preserving the existing controls and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx`:
- Around line 206-223: Update the showTriggerValue column insertion logic near
the existing claimant insertion so claimant is positioned relative to the
duration column rather than a fixed numeric index. Preserve the PRO column order
as event_name, trigger_value, trigger_time, claimant, duration, while leaving
the trigger_value column definition unchanged.
In `@src/pages/alertCurEvent/pages/List/index.tsx`:
- Around line 350-359: Add an accessible name to the Switch in the
show-trigger-value control by setting its aria-label to t('show_trigger_value'),
while preserving the existing checked state and onChange behavior.
- Line 126: Update the showTriggerValue state initializer and its onChange
handler to tolerate localStorage failures: catch read errors and default to
false, and call setShowTriggerValue before attempting the storage write so the
UI updates even when persistence fails. Apply the same handling to both
preference paths associated with SHOW_TRIGGER_VALUE_CACHE_KEY.
---
Nitpick comments:
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx`:
- Line 222: Remove the `as any` cast from the new alert-table column and define
the columns using Ant Design’s `ColumnsType` with the existing alert-record
type, preserving type checking for the column and renderer. Also declare the
component Props through an explicit TypeScript interface and avoid introducing
any `any` types.
In `@src/pages/alertCurEvent/pages/List/index.tsx`:
- Around line 350-375: Replace the new Space wrapper around the show-trigger
Switch and expand Button with a container using Tailwind layout utilities, such
as flex items-center gap-2, while preserving the existing controls and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e34d8eaf-f00a-415d-8826-3ba5ed6ea883
📒 Files selected for processing (8)
src/pages/alertCurEvent/constants.tssrc/pages/alertCurEvent/locale/en_US.tssrc/pages/alertCurEvent/locale/ja_JP.tssrc/pages/alertCurEvent/locale/ru_RU.tssrc/pages/alertCurEvent/locale/zh_CN.tssrc/pages/alertCurEvent/locale/zh_HK.tssrc/pages/alertCurEvent/pages/List/AlertTable.tsxsrc/pages/alertCurEvent/pages/List/index.tsx
| if (showTriggerValue) { | ||
| columns.splice(1, 0, { | ||
| title: t('trigger_value'), | ||
| dataIndex: 'trigger_value', | ||
| fixed: 'right' as const, | ||
| render(value) { | ||
| return ( | ||
| <div | ||
| style={{ | ||
| minWidth: getTextWidth(t('trigger_value')), | ||
| }} | ||
| > | ||
| {value} | ||
| </div> | ||
| ); | ||
| }, | ||
| } as any); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the existing PRO column order.
When showTriggerValue is enabled, the existing PRO insertion at Line 226 places claimant before duration. The resulting order changes from event_name, trigger_time, duration, claimant to event_name, trigger_value, trigger_time, claimant, duration.
Insert claimant relative to the duration column instead of using a fixed numeric index.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/alertCurEvent/pages/List/AlertTable.tsx` around lines 206 - 223,
Update the showTriggerValue column insertion logic near the existing claimant
insertion so claimant is positioned relative to the duration column rather than
a fixed numeric index. Preserve the PRO column order as event_name,
trigger_value, trigger_time, claimant, duration, while leaving the trigger_value
column definition unchanged.
| const [selectedRowKeys, setSelectedRowKeys] = useState<number[]>([]); | ||
| const [alertEscalationEnable, setAlertEscalationEnable] = useState(false); | ||
| const [eventColumnExpanded, setEventColumnExpanded] = useState(() => readAlertEventTagsExpanded(ALERT_CUR_EVENT_TAGS_EXPANDED_TABLE_KEY)); | ||
| const [showTriggerValue, setShowTriggerValue] = useState(() => localStorage.getItem(SHOW_TRIGGER_VALUE_CACHE_KEY) === 'true'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate file and relevant lines:"
if [ -f "src/pages/alertCurEvent/pages/List/index.tsx" ]; then
wc -l src/pages/alertCurEvent/pages/List/index.tsx
echo "--- around line 126 ---"
sed -n '110,135p' src/pages/alertCurEvent/pages/List/index.tsx | nl -ba -v110
echo "--- around lines 340-370 ---"
sed -n '340,370p' src/pages/alertCurEvent/pages/List/index.tsx | nl -ba -v340
else
echo "File not found"
fi
echo "--- search for SHOW_TRIGGER_VALUE_CACHE_KEY usages ---"
rg -n "SHOW_TRIGGER_VALUE_CACHE_KEY|showTriggerValue|setShowTriggerValue" src/pages/alertCurEvent/pages/List/index.tsx src || true
echo "--- locate existing safe storage pattern file mentioned ---"
fd -a "eventColumnExpandedStorage" . | sed 's#^\./##'
if [ -f "src/pages/alertCurEvent/utils/eventColumnExpandedStorage.ts" ]; then
ec=$(fd "eventColumnExpandedStorage" . | head -n1)
wc -l "$ec"
sed -n '1,220p' "$ec" | nl -ba
fi
echo "--- read-only JavaScript behavior probe for localStorage.getItem/setItem throwing ---"
node - <<'JS'
const { JSDOM } = require('jsdom');
for (const blockStorage of [true, false]) {
const vm = new JSDOM('', { url: 'http://localhost/' });
vm.window.blockStorage = () => { vm.window._blockedStorage = true; };
const originalGetItem = vm.window.Storage.prototype.getItem.bind(vm.window.localStorage);
Object.defineProperty(vm.window, 'Storage', {
value: class extends vm.window.Storage {
get localStorage() {
if (vm.window._blockedStorage) {
Object.defineProperty(this, 'localStorage', {
value: new vm.window.Storage(),
writable: true, configurable: true
});
this.localStorage.getItem = () => { throw new DOMException('Storage is disabled.', 'SecurityError'); };
this.localStorage.setItem = () => { throw new DOMException('Storage is disabled.', 'SecurityError'); };
}
return Object.getPrototypeOf(this).localStorage;
}
}
});
try {
const value = originalGetItem('SHOW_TRIGGER_VALUE_CACHE_KEY') === 'true';
console.log(JSON.stringify({ blockStorage, value }));
} catch (e) {
console.log(JSON.stringify({ blockStorage, initialThrows: true, name: e && e.name }));
}
}
JSRepository: n9e/fe
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euxo pipefail
echo "Lines around 110-135:"
sed -n '110,135p' src/pages/alertCurEvent/pages/List/index.tsx
echo "Lines around 340-370:"
sed -n '340,370p' src/pages/alertCurEvent/pages/List/index.tsx
echo "Usages of SHOW_TRIGGER_VALUE_CACHE_KEY, showTriggerValue, setShowTriggerValue:"
rg -n "SHOW_TRIGGER_VALUE_CACHE_KEY|showTriggerValue|setShowTriggerValue" src
echo "eventColumnExpandedStorage files and contents:"
fd "eventColumnExpandedStorage" . -x sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1"' sh {}
echo "jsdom availability:"
node - <<'JS'
try {
require.resolve('jsdom');
console.log('jsdom installed');
} catch (e) {
console.log('jsdom not installed');
}
JSRepository: n9e/fe
Length of output: 9714
🌐 Web query:
MDN HTML5 specification localStorage disabled browsers SecurityError getItem throws
💡 Result:
When localStorage is disabled in a web browser—or when access is restricted due to privacy settings or policy decisions—accessing the localStorage property or calling methods such as getItem will frequently throw a SecurityError DOMException [1][2][3][4]. According to the HTML5 specification and MDN documentation, a SecurityError is thrown if the request violates a policy decision, such as when a user agent is configured to prevent the page from persisting data [1][3]. While the behavior can be browser-dependent, many browsers (including Chrome and Firefox) effectively link localStorage access to cookie settings [5][6]. If a user blocks cookies or configures their browser to disallow site data, the browser may interpret this as a directive to disable localStorage, causing attempts to access or use it to fail with a SecurityError [1][4][5][6]. Because simply checking for the existence of window.localStorage is not a reliable way to determine if it is usable (as the property may exist but access to it may still be denied), it is widely recommended to wrap any localStorage interaction in a try...catch block to gracefully handle the potential SecurityError [7][5][8][9]. A common pattern for feature detection is: try { localStorage.setItem('test', 'test'); localStorage.removeItem('test'); } catch (e) { // localStorage is disabled or otherwise unavailable } [8]
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage?gn=1
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage
- 3: https://html.spec.whatwg.org/multipage/webstorage.html
- 4: https://www.chromium.org/for-testers/bug-reporting-guidelines/uncaught-securityerror-failed-to-read-the-localstorage-property-from-window-access-is-denied-for-this-document/
- 5: https://blog.tomayac.com/2022/08/30/things-not-available-when-someone-blocks-all-cookies/
- 6: https://stackoverflow.com/questions/26550770/can-session-storage-local-storage-be-disabled-and-cookies-enabled
- 7: https://stackoverflow.com/questions/30481516/iframe-in-chrome-error-failed-to-read-localstorage-from-window-access-deni
- 8: https://stackoverflow.com/questions/16427636/check-if-localstorage-is-available
- 9: https://michalzalecki.com/why-using-localStorage-directly-is-a-bad-idea/
Handle localStorage failures in both preference paths.
Line 126 reads localStorage during React state initialization; blocking storage can throw and prevent the page from rendering. The onChange path also calls localStorage.setItem before setShowTriggerValue, so a write failure leaves the switch behind the in-memory value. Guard both accesses: default to false on read failure, and update the switch first so writes are ignorable.
Also applies to: 350-358
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/alertCurEvent/pages/List/index.tsx` at line 126, Update the
showTriggerValue state initializer and its onChange handler to tolerate
localStorage failures: catch read errors and default to false, and call
setShowTriggerValue before attempting the storage write so the UI updates even
when persistence fails. Apply the same handling to both preference paths
associated with SHOW_TRIGGER_VALUE_CACHE_KEY.
| <Space> | ||
| <span className='text-sm text-fc-text-3'>{t('show_trigger_value')}</span> | ||
| <Switch | ||
| size='small' | ||
| className='alert-event-expand-btn' | ||
| icon={eventColumnExpanded ? <ListChevronsDownUp size={14} /> : <ListChevronsUpDown size={14} />} | ||
| onClick={() => { | ||
| setEventColumnExpanded((expanded) => { | ||
| const next = !expanded; | ||
| writeAlertEventTagsExpanded(ALERT_CUR_EVENT_TAGS_EXPANDED_TABLE_KEY, next); | ||
| return next; | ||
| }); | ||
| checked={showTriggerValue} | ||
| onChange={(checked) => { | ||
| localStorage.setItem(SHOW_TRIGGER_VALUE_CACHE_KEY, String(checked)); | ||
| setShowTriggerValue(checked); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file and surrounding code.
target="src/pages/alertCurEvent/pages/List/index.tsx"
if [ -f "$target" ]; then
echo "=== file exists ==="
wc -l "$target"
echo
echo "=== relevant lines 330-380 ==="
sed -n '330,380p' "$target" | nl -ba -v330
echo
echo "=== surrounding imports/declarations for SHOW_TRIGGER_VALUE_CACHE_KEY and showTriggerValue ==="
rg -n "SHOW_TRIGGER_VALUE_CACHE_KEY|showTriggerValue|show_trigger_value|Switch|<Space>" "$target"
else
echo "target file not found"
fd -a 'index\.tsx$' . | sed -n '1,80p'
fiRepository: n9e/fe
Length of output: 286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target="src/pages/alertCurEvent/pages/List/index.tsx"
echo "=== relevant lines 330-380 ==="
sed -n '330,380p' "$target"
echo
echo "=== relevant declarations/usages ==="
rg -n "SHOW_TRIGGER_VALUE_CACHE_KEY|showTriggerValue|show_trigger_value|Switch|<Space>" "$target"Repository: n9e/fe
Length of output: 3675
Give the switch a name.
The span next to the Switch is only a visual label; it is not connected to the control. Add aria-label={t('show_trigger_value')} or use an explicit label association.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/alertCurEvent/pages/List/index.tsx` around lines 350 - 359, Add an
accessible name to the Switch in the show-trigger-value control by setting its
aria-label to t('show_trigger_value'), while preserving the existing checked
state and onChange behavior.
feat 解决issue#2873,在活跃告警页面新增“显示触发时值”开关,打开后能无需点击明细在界面直接显示触发时值,开关由localstorage中变量名为alert_cur_events_show_trigger_value的布尔值控制

Summary by CodeRabbit