Skip to content

feat: display rules used #5677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
May 15, 2025
Merged

feat: display rules used #5677

merged 19 commits into from
May 15, 2025

Conversation

tomasz-stefaniak
Copy link
Collaborator

@tomasz-stefaniak tomasz-stefaniak commented May 14, 2025

Description

display rules used

Checklist

  • I've read the contributing guide
  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

Screenshots

image

Tests

[ What tests were added or updated to ensure the changes work as expected? ]

@tomasz-stefaniak tomasz-stefaniak requested a review from a team as a code owner May 14, 2025 23:32
@tomasz-stefaniak tomasz-stefaniak requested review from Patrick-Erichsen and removed request for a team May 14, 2025 23:32
Copy link

netlify bot commented May 14, 2025

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 002ecc9
🔍 Latest deploy log https://app.netlify.com/projects/continuedev/deploys/68265691e69b5b0008b34a8c

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 14, 2025
Copy link

github-actions bot commented May 14, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@tomasz-stefaniak
Copy link
Collaborator Author

I have read the CLA Document and I hereby sign the CLA

@tomasz-stefaniak
Copy link
Collaborator Author

recheck

github-actions bot added a commit that referenced this pull request May 15, 2025
Copy link
Collaborator

@Patrick-Erichsen Patrick-Erichsen left a comment

Choose a reason for hiding this comment

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

  • There is some jank when both rules + context items are used
    • The rule peek is rendered first, then the context item is rendered above it, which causes jank. We can fix just by always rendering rules first since context items has lag
  • Gap between context item + rules peek is a bit too big imo
  • Can we add an affordance to click on the rule somewhere and view the file? Like we do in the notch
image
  • I think we should only give ~1/3vh to the rules peek, right now appears to be closer to 2/3
  • In a world where there are tens of rules, each potentially quite long, I lean towards setting a line-clamp-3 (or maybe a few more lines) for individual rules. Imo the name of the rule is the most important thing, if you want the full text click on it and go to the file
  • Can we make the "Applies to all files" text color more subtle, like the "Source" text? Feels like it's drawing too much attention since it's quite close to the rule title
  • It looks like newlines aren't being preserved
  • "Global" is a bit vague to me. I think it's the right term but seeing it next to "Applies to all files" is confusing. Adding an info icon with hover text to explain what "Global" means would solve it imo

>
<div className="flex w-full items-center">
{isGlobal ? (
<GlobeAltIcon className="mr-2 h-4 w-4 flex-shrink-0 text-gray-400" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was a bit confusing to me when I only had global rules. Given we already have the "Global Rules" header, and the "Applies to all files" text, I lean towards just using the same PencilIcon that we have in the notch for all rules.

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs May 15, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 15, 2025
@tomasz-stefaniak tomasz-stefaniak merged commit 680887a into main May 15, 2025
35 checks passed
@tomasz-stefaniak tomasz-stefaniak deleted the tomasz/display-rules branch May 15, 2025 21:22
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs May 15, 2025
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants