feat(instruments): add draft-trim instrument - #1066
Conversation
New obc-draft-trim navigation instrument: aft and fore draft on two vertical linear gauges (watchfaceLinear building block) around a side-profile vessel image tilted by a trim property. Regular and enhanced priority variants per the OpenBridge 6.1 Figma design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds the ChangesDraft-trim instrument
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ObcDraftTrim
participant watchfaceLinear
participant SVG
ObcDraftTrim->>watchfaceLinear: Build fore and aft gauges from draft values
watchfaceLinear-->>ObcDraftTrim: Return gauge renderings
ObcDraftTrim->>SVG: Apply vessel trim, scale, and gauge transforms
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.ts (1)
18-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a Best Practices / Constraints section.
Document practical constraints, especially that the range must be positive and that the selected image should be a side profile. As per coding guidelines, component documentation must include practical best practices and constraints.
🤖 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 `@packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.ts` around lines 18 - 38, Extend the Draft-trim instrument documentation comment with a “Best Practices / Constraints” section covering that instrumentRange must be positive and the configured vessel image should be a side-profile view. Keep the guidance specific to the component’s existing range and image configuration.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
`@packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.stories.ts`:
- Around line 43-45: Rename the exported story constant in the draft-trim story
from Regular to Default, preserving its current args and Story type.
In
`@packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.ts`:
- Line 46: The public instrumentRange property in the draft-trim component must
accept only positive finite numbers. Normalize or reject zero, negative, NaN,
and other non-finite values before the watchfaceLinear call and ensure
minValue/maxValue always receive a valid positive range; apply this validation
to the property handling and the related range calculations around the
identified symbols.
---
Nitpick comments:
In
`@packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.ts`:
- Around line 18-38: Extend the Draft-trim instrument documentation comment with
a “Best Practices / Constraints” section covering that instrumentRange must be
positive and the configured vessel image should be a side-profile view. Keep the
guidance specific to the component’s existing range and image configuration.
🪄 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: 76e952d0-bcd8-4859-9522-67c60a448968
📒 Files selected for processing (2)
packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.stories.tspackages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.ts
| export const Regular: Story = { | ||
| args: {}, | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Export the base story as Default.
New navigation-instrument story files must expose a Default story; rename Regular to Default while keeping its current args.
Based on learnings, new or updated navigation-instrument stories must export Default rather than Regular.
Proposed fix
-export const Regular: Story = {
+export const Default: Story = {
args: {},
};📝 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.
| export const Regular: Story = { | |
| args: {}, | |
| }; | |
| export const Default: Story = { | |
| args: {}, | |
| }; |
🤖 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
`@packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.stories.ts`
around lines 43 - 45, Rename the exported story constant in the draft-trim story
from Regular to Default, preserving its current args and Story type.
Source: Learnings
There was a problem hiding this comment.
Skipping the rename: navigation-instruments has no Default-first convention in practice - depth-actual and heave export Regular first, and siblings like pitch-roll/pitch/roll use Primary. Keeping Regular matches the closest existing instruments.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Locate repository guidance =='
fd -HI -t f '^(AGENTS|IMPLEMENTATION_GUIDELINES)\.md$' . || true
printf '%s\n' '== Relevant story exports =='
rg -n --glob '*.stories.ts' \
'export const (Default|Regular|Primary)\b' \
packages/openbridge-webcomponents/src/navigation-instruments \
| rg '(depth-actual|heave|pitch-roll|pitch|roll|draft-trim)\.stories\.ts' || true
printf '%s\n' '== Guidance references to story naming =='
rg -n -i -C 3 \
'(storybook|stories|story).*default|default.*(storybook|stories|story)|export.*Default' \
AGENTS.md IMPLEMENTATION_GUIDELINES.md 2>/dev/null || trueLength of output: 2748
@ludvigovrevik, I verified the cited sibling stories—they do use Regular/Primary. However, both written guidelines explicitly require a Default export for new stories: AGENTS.md Line 326 and IMPLEMENTATION_GUIDELINES.md Line 32.
Since this is a new story module, the finding remains valid; the closest existing examples appear to be historical inconsistencies rather than the prescribed convention.
🧠 Learnings used
Learnt from: talpitoo
Repo: Ocean-Industries-Concept-Lab/openbridge-webcomponents PR: 1058
File: packages/openbridge-webcomponents/src/navigation-instruments/draft-trim/draft-trim.stories.ts:0-0
Timestamp: 2026-07-28T11:34:42.167Z
Learning: For new or updated Storybook story files under packages/openbridge-webcomponents/src/navigation-instruments (e.g., *.stories.ts), ensure the exported stories follow the written convention in AGENTS.md §5 and IMPLEMENTATION_GUIDELINES.md by including a story export named `Default`. Do not rely on historical inconsistencies (e.g., `Primary`/`Regular`/`InCommand`)—even if the component’s behavior is related to another component that uses a different story name.
Learnt from: ludvigovrevik
Repo: Ocean-Industries-Concept-Lab/openbridge-webcomponents PR: 1065
File: packages/openbridge-webcomponents/src/navigation-instruments/pitch-roll-yaw/pitch-roll-yaw.stories.ts:56-58
Timestamp: 2026-07-28T13:13:35.500Z
Learning: When reviewing Storybook story files under `packages/openbridge-webcomponents/src/navigation-instruments`, require that new/updated component stories export `Default` (not `Primary`) in accordance with `AGENTS.md` and `IMPLEMENTATION_GUIDELINES.md`. Do not use nearby/history examples that export `Primary` as precedent; the repo guidelines take precedence.
|
The demo can be viewed at https://openbridge-next-demo--1066-feat-instruments-add-draft-o20isewd.web.app |
|
The storybook can be viewed at https://openbridge-next-storybook--1066-feat-instruments-add-d-8vovflf3.web.app |
CI format:check failed; the PR was committed from a temp worktree where the pre-commit hook could not run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Zero, negative or NaN ranges produced a degenerate or reversed minValue/maxValue domain; fall back to the default 10, mirroring pitch-roll-yaw's safeRange guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
talpitoo
left a comment
talpitoo
left a comment
There was a problem hiding this comment.
hey @ludvigovrevik this instrument is already part of a bigger #1058 where a did more rounds of code review, documentation, etc. I am going to request 'blank changes' to avoid accidental merge but we can keep it open for reference.
talpitoo
left a comment
talpitoo
left a comment
There was a problem hiding this comment.
@ludvigovrevik this is probably WIP since the snapshots are missing but the component was implemented in #1058 anyway so perhaps we close this?
|
Yes, I think we should close this one since its a part of #1058. |
3 participants
What
New
obc-draft-trimnavigation instrument implementing the OpenBridge 6.1 Draft-trim Figma design (node 20122-231157), Priority=False / Enhanced variants.watchfaceLinearbuilding block, same asobc-heave/obc-depth-actual/ feat(instruments): add pitch-roll-heave instrument #1064. Fill runs from the waterline down to the draft value with a rounded bar marker.vesselImages, defaultpsvSide) rotated by atrimproperty (degrees, positive = bow down — same sign convention aspitchinobc-pitch-roll).priority: Priorityswaps regular/enhanced semantic instrument tokens (handled insidewatchfaceLinear); all colors are semantic CSS variables, no hex.draftAft,draftFore,trim,instrumentRange,primaryTickmarkInterval,secondaryTickmarkInterval,vesselImage,vesselScale,priority.['autodocs', 'wip', 'skip-test']— no visual baselines yet while in development.Design decisions (for review)
trimis an explicit property rather than derived from the draft difference (physical trim angle would require vessel length).minAvg*/maxAvg*trend bands — the design shows none; the gauge box slot is used for the waterline→draft fill itself.Validation
npx tsc --noEmitcleaneslint src/navigation-instruments/draft-trim --max-warnings 0cleannpm run analyze—obc-draft-trimpresent in the manifest🤖 Generated with Claude Code
Summary by CodeRabbit
obc-draft-trimnavigation instrument web component that renders fore and aft draft gauges, a trim angle visualization, and configurable tick/scale settings.