Skip to content

feat: active alerts display values . from issue #2873 - #2236

Closed
zhen1995 wants to merge 1 commit into
n9e:mainfrom
zhen1995:feature-issues/2873
Closed

feat: active alerts display values . from issue #2873#2236
zhen1995 wants to merge 1 commit into
n9e:mainfrom
zhen1995:feature-issues/2873

Conversation

@zhen1995

@zhen1995 zhen1995 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

feat 解决issue#2873,在活跃告警页面新增“显示触发时值”开关,打开后能无需点击明细在界面直接显示触发时值,开关由localstorage中变量名为alert_cur_events_show_trigger_value的布尔值控制
switch-in-localstorage

show-alert-value

Summary by CodeRabbit

  • New Features
    • Added an option to show alert trigger values in the current-events table.
    • Added a toolbar switch to turn trigger-value visibility on or off.
    • Your display preference is remembered between visits.
  • Localization
    • Added translated labels for the trigger value and visibility setting in English, Japanese, Russian, Simplified Chinese, and Hong Kong Chinese.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The alert current-events page adds a persisted switch for showing trigger values. AlertTable conditionally renders a translated, right-fixed trigger-value column. Locale entries were added for six languages.

Changes

Alert trigger value display

Layer / File(s) Summary
Preference control and localization
src/pages/alertCurEvent/constants.ts, src/pages/alertCurEvent/locale/*, src/pages/alertCurEvent/pages/List/index.tsx
The page adds the SHOW_TRIGGER_VALUE_CACHE_KEY constant, localized labels, local-storage initialization, and a toolbar switch that persists showTriggerValue.
Trigger-value column rendering
src/pages/alertCurEvent/pages/List/AlertTable.tsx, src/pages/alertCurEvent/pages/List/index.tsx
AlertTable receives showTriggerValue and conditionally adds the fixed trigger_value column. The page passes the setting to the table.

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
Loading

Possibly related PRs

  • n9e/fe#2233: Both changes persist separate alert-page settings in local storage.

Suggested reviewers: jsers

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the main change: displaying values in active alerts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch feature-issues/2873
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/pages/alertCurEvent/pages/List/AlertTable.tsx (1)

222-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the new column type-safe.

The as any cast at Line 222 suppresses column and renderer type checking. Use the existing alert-record type with an Ant Design ColumnsType definition, 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 win

Use Tailwind for the new toolbar container.

The new Space controls flex and gap layout. Replace it with a small container using flex items-center gap-2 or 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

📥 Commits

Reviewing files that changed from the base of the PR and between de7d470 and d6c38d5.

📒 Files selected for processing (8)
  • src/pages/alertCurEvent/constants.ts
  • src/pages/alertCurEvent/locale/en_US.ts
  • src/pages/alertCurEvent/locale/ja_JP.ts
  • src/pages/alertCurEvent/locale/ru_RU.ts
  • src/pages/alertCurEvent/locale/zh_CN.ts
  • src/pages/alertCurEvent/locale/zh_HK.ts
  • src/pages/alertCurEvent/pages/List/AlertTable.tsx
  • src/pages/alertCurEvent/pages/List/index.tsx

Comment on lines +206 to +223
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);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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 }));
  }
}
JS

Repository: 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');
}
JS

Repository: 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:


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.

Comment on lines +350 to 359
<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);
}}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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'
fi

Repository: 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant