Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 1, 2025

Addressing PR feedback comments:

  • Delete REPORT_FEATURE.md file
  • Delete package-lock.json (use pnpm only)
  • Fix error handling in inspect-url.post.ts to throw instead of silent fallback
  • Make URL source of truth - navigate to /report/:reportId after creating report
  • Fix test typing for $fetch global
  • Move reportId from ApiRun to Run type
  • Update permalink label to "Share this report:"
  • Make permalink input wider
  • Fix button vertical alignment and use border styling instead of background

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits July 1, 2025 23:49
Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com>
Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com>
Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com>
Copilot AI changed the title [WIP] Generate permalinks for a set of runs Implement report permalinks for sharing sets of runs Jul 2, 2025
Copilot AI requested a review from serhalp July 2, 2025 00:02
Copilot finished work on behalf of serhalp July 2, 2025 00:02
Copy link
Owner

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

@copilot seems to work, but see feedback

Comment on lines +61 to +65
<input
:value="generateReportPermalink(currentReportId)"
readonly
class="permalink-input"
/>
Copy link
Owner

Choose a reason for hiding this comment

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

This should be much wider as it often contains quite long URLs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in b38cbb7 - changed max-width from 600px to 100% so the input can expand to show longer URLs.

Comment on lines +66 to +72
<button
class="copy-button"
title="Copy to clipboard"
@click="copyToClipboard(generateReportPermalink(currentReportId))"
>
📋
</button>
Copy link
Owner

Choose a reason for hiding this comment

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

for some reason this isn't properly vertically aligned with the input

Image

I also find the blue background rather ugly. must just a sleek blue border instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b38cbb7:

  • Changed align-items from center to stretch to ensure proper vertical alignment
  • Replaced blue background with white background and 2px blue border (border: 2px solid var(--blue-600))
  • Added display: flex with align-items: center on button for consistent internal alignment
  • On hover, button now shows light blue background (var(--blue-50)) instead of darker blue

@serhalp serhalp changed the title Implement report permalinks for sharing sets of runs feat: store sets of runs as shareable reports Nov 23, 2025
@serhalp serhalp marked this pull request as ready for review November 23, 2025 13:40
… and update UI

Co-authored-by: serhalp <1377702+serhalp@users.noreply.github.com>
Copilot AI requested a review from serhalp November 23, 2025 14:01
Copilot finished work on behalf of serhalp November 23, 2025 14:01
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.

2 participants