fix(automation-button): forward alert frame slots through abstract button - #995
Draft
tibnor wants to merge 4 commits into
Draft
fix(automation-button): forward alert frame slots through abstract button#995tibnor wants to merge 4 commits into
tibnor wants to merge 4 commits into
Conversation
…tton The abstract automation button rendered the underlying obc-automation-button but only forwarded the badge slots. Slotted alert-frame content (icon, label, timer) placed on a device such as obc-analog-valve never reached the alert frame, so it could not be shown. Forward the slots all the way down, remapping the public alert-frame-icon / alert-frame-label / alert-frame-timer slots onto the automation button's alert-icon / alert-label / alert-timer slots (which in turn feed the alert frame's icon / label / timer slots). Document these slots on ObcAbstractAutomationButton rather than on each device component. Add an analog valve story that shows a bottom-flip alert frame with a custom icon, an alert label and a clock, and add its visual baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWMYJ6JF3jpspmmoQMNt6m
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough
ChangesAlert-frame slot forwarding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/openbridge-webcomponents/src/automation/analog-valve/analog-valve.stories.ts`:
- Around line 68-73: Remove the story-level JSDoc block comment from the
analog-valve.stories.ts file that starts with "The analog valve forwards..." and
explains the alert frame slots and bottom-flip frame type. This multi-line
comment block (beginning with /** and ending with */) should be completely
deleted as it is self-explanatory and conflicts with the repository's coding
guidelines that restrict comments to only necessary cases for unusual or complex
code.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45fe93bf-ab32-4c60-8c67-cbfc93ecce2c
⛔ Files ignored due to path filters (1)
packages/openbridge-webcomponents/__vis__/linux/__baselines__/automation/analog-valve/analog-valve.stories.ts/alert-frame-with-icon-label-and-clock-auto.pngis excluded by!**/*.png
📒 Files selected for processing (2)
packages/openbridge-webcomponents/src/automation/analog-valve/analog-valve.stories.tspackages/openbridge-webcomponents/src/automation/automation-button/abstract-automation-button.ts
|
The demo can be viewed at https://openbridge-next-demo--995-fix-automation-button-forwar-5jsxwe4z.web.app |
|
The storybook can be viewed at https://openbridge-next-storybook--995-fix-automation-button-f-8wjhvp5e.web.app |
…ories Switch the analog valve alert story to a large-side-flip frame with a pressure icon, which suits a valve alarm and fits the valve's small symbol (the side flap has room for an icon but not a label/clock). Add an equivalent tank story using a bottom-flip frame, which the larger tank has room for: it shows a tank icon, a label and a clock via the tank's alert-icon / alert-label / alert-timer slots. Update the visual baselines accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWMYJ6JF3jpspmmoQMNt6m
tibnor
marked this pull request as draft
June 21, 2026 19:12
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/openbridge-webcomponents/src/automation/automation-tank/automation-tank.stories.ts`:
- Around line 489-495: Remove the JSDoc comment block that explains the alarm
alert frame story functionality. This multi-line comment block documenting the
story's behavior violates the repository's coding guideline that prohibits
comments in TypeScript/JavaScript files unless the code is extremely unusual and
impossible to understand without explanation. Delete the entire comment block
starting with the opening /** and ending with the closing */ that precedes the
story implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51113edf-793b-4f08-af33-8228dda5be61
⛔ Files ignored due to path filters (2)
packages/openbridge-webcomponents/__vis__/linux/__baselines__/automation/analog-valve/analog-valve.stories.ts/alert-frame-with-icon-auto.pngis excluded by!**/*.pngpackages/openbridge-webcomponents/__vis__/linux/__baselines__/automation/automation-tank/automation-tank.stories.ts/with-alert-bottom-flip-auto.pngis excluded by!**/*.png
📒 Files selected for processing (2)
packages/openbridge-webcomponents/src/automation/analog-valve/analog-valve.stories.tspackages/openbridge-webcomponents/src/automation/automation-tank/automation-tank.stories.ts
Remove the description comments above the new analog valve and tank alert stories to follow the repository's no-comments rule for TS files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWMYJ6JF3jpspmmoQMNt6m
…ning types Rename the device-level alert-frame slots to alert-icon / alert-label / alert-timer so they match the slot names already used by obc-automation-button (and obc-automation-tank). Previously the alert-icon / alert-label / alert-timer slots were only exposed for `point` positioning; the `symbol` and `button` paths wrapped the content with wrapWithAlertFrame, which did not forward the slots. Render the alert frame through a shared renderAlertFrame() helper in both paths so slotted icon, label and timer content reaches the frame for every positioning value. Update the analog valve story and add an automation-button story that exercises the slots with `button` positioning, plus its visual baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWMYJ6JF3jpspmmoQMNt6m
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The abstract automation button rendered the underlying obc-automation-button
but only forwarded the badge slots. Slotted alert-frame content (icon, label,
timer) placed on a device such as obc-analog-valve never reached the alert
frame, so it could not be shown.
Forward the slots all the way down, remapping the public alert-frame-icon /
alert-frame-label / alert-frame-timer slots onto the automation button's
alert-icon / alert-label / alert-timer slots (which in turn feed the alert
frame's icon / label / timer slots). Document these slots on
ObcAbstractAutomationButton rather than on each device component.
Add an analog valve story that shows a bottom-flip alert frame with a custom
icon, an alert label and a clock, and add its visual baseline.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XWMYJ6JF3jpspmmoQMNt6m
Summary by CodeRabbit
Release Notes
New Features
Documentation