Skip to content

feat(agents): issue-36-fix-web-entered-city-missing-from-live-p - #40

Merged
mctl-claude-remote[bot] merged 2 commits into
mainfrom
feat/agents-issue-36-fix-web-entered-city-missing-from-live-p
Jun 3, 2026
Merged

mctl-claude-remote[bot] merged 2 commits into
mainfrom
feat/agents-issue-36-fix-web-entered-city-missing-from-live-p

Conversation

@mashkovd

@mashkovd mashkovd commented Jun 3, 2026

Copy link
Copy Markdown
Member

Implements accepted proposal mctl-pairdesk/issue-36-fix-web-entered-city-missing-from-live-p.

Spec: https://github.com/mctlhq/mctl-gitops/tree/main/platform-gitops/agents-state/mctl-pairdesk/proposals/issue-36-fix-web-entered-city-missing-from-live-p/

Generated by mctl-agents Tier 2 implementer. Review carefully — this PR was opened automatically and the spec may have gaps.

Closes #36

Decouple location_city rendering from the Maker component in the
outcome and rate variants of OrderCard. Previously city was passed
as the Maker sub prop, which caused it to be silently discarded
whenever maker is null (e.g. the Create-Order live preview). City
is now rendered as a sibling pd-loc span, consistent with the
existing standard variant.

Proposal: platform-gitops/agents-state/mctl-pairdesk/proposals/issue-36-fix-web-entered-city-missing-from-live-p/
@claude

claude Bot commented Jun 3, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 1m 22s —— View job


PR Review: feat(agents): issue-36-fix-web-entered-city-missing-from-live-p

Status: Request Changes — P2 findings

Summary

This PR refactors how the location_city is rendered in two OrderCard variants (outcome and rate), moving it from a sub-item within the Maker component to an independent footer element. The layout change is sound and makes these variants consistent with the standard variant, BUT the corresponding CSS styling is missing.

Findings

P2 — Missing CSS styling:

  • The outcome and rate variants now render .pd-loc elements (lines 315–318, 402–406), but styles.css only defines styling for .pd-card-standard .pd-loc (line 301)
  • Without .pd-card-outcome .pd-loc and .pd-card-rate .pd-loc rules, the city icon + text will not be styled correctly in those variants
  • Fix: Add the missing CSS rules to styles.css

Comment thread web/src/components.tsx
Comment on lines +315 to +318
{order.location_city && (
<span className="pd-loc">
<Icon name="pin" size={12} cls="pd-mut-ic" />{order.location_city}
</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The pd-loc class is being used here, but CSS styling for .pd-card-outcome .pd-loc is missing in styles.css. Currently only .pd-card-standard .pd-loc is defined (line 301). This will cause the city icon + text to be unstyled in the outcome variant. Same issue in the rate variant below (line 404).

Add to styles.css:

.pd-card-outcome .pd-loc { display: inline-flex; align-items: center; gap: 3px; color: var(--pd-hint); }
.pd-card-rate .pd-loc    { display: inline-flex; align-items: center; gap: 3px; color: var(--pd-hint); }

@claude claude 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.

Has P2 findings, changes requested: missing CSS styling for .pd-loc in outcome and rate card variants. The code renders city with pd-loc class but only .pd-card-standard .pd-loc is styled in styles.css. Add corresponding rules for .pd-card-outcome and .pd-card-rate to fix layout/visibility.

Address claude-review P2 on PR #40: the city was moved to an independent
.pd-loc footer element in the outcome and rate OrderCard variants, but only
.pd-card-standard .pd-loc had CSS. Add the matching .pd-card-outcome .pd-loc
and .pd-card-rate .pd-loc rules so the pin icon + city render consistently.
@claude

claude Bot commented Jun 3, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 44s —— View job


PR Review: feat(agents): issue-36-fix-web-entered-city-missing-from-live-p

Status

✅ APPROVED — P2 findings resolved

Summary

This PR refactors how location_city is rendered in the outcome and rate OrderCard variants, moving it from a nested element within the Maker component to an independent footer element. The previous review identified missing CSS styling for these variants, and this update has addressed that finding.

Verification

Previous P2 Finding: Missing CSS styling for .pd-card-outcome .pd-loc and .pd-card-rate .pd-loc

Current State: ✅ Fixed

  • Line 329 in styles.css: .pd-card-rate .pd-loc now defined
  • Line 428 in styles.css: .pd-card-outcome .pd-loc now defined
  • Both rules use consistent styling: display: inline-flex; align-items: center; gap: 3px; color: var(--pd-hint);
  • Matches the standard variant's .pd-loc styling (line 301)

Component Changes: ✅ Correct

  • Outcome variant (lines 315–319 in components.tsx): city rendered as independent .pd-loc element
  • Rate variant (lines 402–406 in components.tsx): city rendered as independent .pd-loc element
  • Both properly use the .pd-loc class that is now styled in CSS

Conclusion

All previous P2 findings have been resolved. The CSS styling is in place and consistent across all card variants. The layout refactoring maintains visual consistency while improving component separation of concerns.

No P1/P2 findings. Good to merge.

@claude claude 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.

No P1/P2 findings. Good to merge.

@mctl-claude-remote mctl-claude-remote Bot added the steward:owned pr-steward owned label Jun 3, 2026
@mctl-claude-remote
mctl-claude-remote Bot merged commit 271c28e into main Jun 3, 2026
5 checks passed
@mctl-claude-remote
mctl-claude-remote Bot deleted the feat/agents-issue-36-fix-web-entered-city-missing-from-live-p branch June 3, 2026 20:19
@mctl-claude-remote mctl-claude-remote Bot added steward:merged pr-steward merged and removed steward:owned pr-steward owned labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

steward:merged pr-steward merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(web): entered city missing from live preview card

2 participants