Skip to content

feat: add landmarks to health chart#210

Open
graphieros wants to merge 10 commits into
MatteoGabriele:mainfrom
graphieros:health-chart-landmarks
Open

feat: add landmarks to health chart#210
graphieros wants to merge 10 commits into
MatteoGabriele:mainfrom
graphieros:health-chart-landmarks

Conversation

@graphieros

@graphieros graphieros commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

This adds a landmark on the chart to signal the packages sample update(s).
It is designed to become visible when the date of the landmark is a week older than the last datapoint.

Here is a screenshot with the landmark positioned at an earlier date so it is visible for the demo:

image

Tooltip shows the details of the landmark when hovering its index:

image

This is probably a temporary feature, designed to facilitate the before/after trend analysis.

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features
    • Added a temporary “landmarks” overlay to the XY chart, showing vertical reference lines with rotated labels for key dates.
    • Updated chart tooltips to include a “LANDMARK INFO” section with the landmark’s name and description when the landmark is currently eligible.
    • Landmark visibility is limited to entries that fall within a one-week window before the end of the loaded dataset.

@netlify

netlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deploy Preview for agentscan ready!

Name Link
🔨 Latest commit e5eaefe
🔍 Latest deploy log https://app.netlify.com/projects/agentscan/deploys/6a3e13d6964b050008c48b4a
😎 Deploy Preview https://deploy-preview-210--agentscan.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc65a4f0-4c61-4ba6-8eda-29220eeac9d8

📥 Commits

Reviewing files that changed from the base of the PR and between e02c8b5 and b51054e.

📒 Files selected for processing (1)
  • app/components/Chart/GlobalEventsEvolution.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/components/Chart/GlobalEventsEvolution.vue

📝 Walkthrough

Walkthrough

GlobalEventsEvolution.vue adds a temporary landmark overlay tied to chart dates. It computes visible landmark metadata from the dataset, draws landmark markers in the SVG, and shows landmark details in the tooltip for matching datapoints.

Changes

Chart Landmarks Overlay

Layer / File(s) Summary
Landmark data and visibility
app/components/Chart/GlobalEventsEvolution.vue
Defines hardcoded landmarks, resolves each one to a chart date index, applies the one-week visibility rule, and builds lookup data by datapoint index.
SVG markers and tooltip info
app/components/Chart/GlobalEventsEvolution.vue
Draws vertical landmark markers with labels in the SVG layer and conditionally renders landmark name/description in the tooltip for the active datapoint.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • MatteoGabriele

Poem

🐇 I hopped where charted dates align,
And spotted landmarks in a line.
A marker, label, tooltip glow,
For points that match the path below.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding landmarks to the health chart.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@graphieros graphieros marked this pull request as draft June 23, 2026 15:39

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

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 `@app/components/Chart/GlobalEventsEvolution.vue`:
- Around line 275-277: The v-for loop iterating through keyDates using landmark
and index j currently binds the key to landmark?.name which is not guaranteed to
be unique and can cause incorrect vnode reuse in Vue when duplicate landmark
names exist. Replace the :key binding in the v-for directive to use a unique
identifier by either using the index j directly or combining the landmark name
with its index position to ensure uniqueness across all iterations.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: c264624a-ce93-4ad9-bf8d-66c20305457e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ddb834 and 50947db.

📒 Files selected for processing (1)
  • app/components/Chart/GlobalEventsEvolution.vue

Comment thread app/components/Chart/GlobalEventsEvolution.vue
@graphieros graphieros marked this pull request as ready for review June 23, 2026 16:06
@graphieros graphieros marked this pull request as draft June 23, 2026 16:06
@graphieros graphieros marked this pull request as ready for review June 26, 2026 05:48

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

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 `@app/components/Chart/GlobalEventsEvolution.vue`:
- Around line 222-225: The sample event date in GlobalEventsEvolution.vue is
still using the demo landmark instead of the production one; update the date
field in the relevant event object from the placeholder value to the real sample
update date referenced by the inline FIXME so the chart uses the correct
before/after window. Use the event entry containing name "Sample update" and
description "13 repositories added to the dataset" to locate it, and make sure
no demo-only date remains in that dataset.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 15b1b059-e0c4-41ed-97c5-5da43138fe5a

📥 Commits

Reviewing files that changed from the base of the PR and between 50947db and 4b96d1a.

📒 Files selected for processing (1)
  • app/components/Chart/GlobalEventsEvolution.vue

Comment thread app/components/Chart/GlobalEventsEvolution.vue Outdated
@graphieros graphieros force-pushed the health-chart-landmarks branch from e02c8b5 to 9c812f6 Compare June 28, 2026 14:44

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

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 `@app/components/Chart/GlobalEventsEvolution.vue`:
- Line 222: The single-line guard clauses in GlobalEventsEvolution.vue are
violating the repo’s curly lint rule. Update the affected `if` statements to use
braces in the relevant logic around `lastDate` and the other matching guard at
the referenced spot, keeping the same behavior but wrapping each conditional
branch in a block.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: abddb538-ec81-4591-9493-04d5888fae27

📥 Commits

Reviewing files that changed from the base of the PR and between 4b96d1a and e02c8b5.

📒 Files selected for processing (1)
  • app/components/Chart/GlobalEventsEvolution.vue

Comment thread app/components/Chart/GlobalEventsEvolution.vue Outdated
@MatteoGabriele

Copy link
Copy Markdown
Owner

How do you plan to drop this one?

@graphieros

Copy link
Copy Markdown
Collaborator Author

How do you plan to drop this one?

It needs your final touch on:

  • landmark label
  • toggle hide/show annotations ?
  • do we keep a single landmark as in the PR, or do we add the increase to 300 queries we had recently ?

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