Skip to content
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

Add scroll to Event ID for links to Events #2541

Merged
merged 11 commits into from
Feb 24, 2025
Merged

Conversation

Alex-Tideman
Copy link
Contributor

@Alex-Tideman Alex-Tideman commented Feb 6, 2025

Description & motivation 💭

Allow users to navigate to the history page with a specific event .../history#138 - and scroll to it. It will not open the row by default, just scroll to it.

Also add full event group on specific event page /history/event/138.

Screenshots (if applicable) 📸

ScrollToId.mov

Design Considerations 🎨

Testing 🧪

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

Checklists

Draft Checklist

Merge Checklist

  • Need to figure out how to wait on page change before scrolling if event is not on current page

Issue(s) closed

Docs

Any docs updates needed?

…change before scrolling if event id is not on the current page
Copy link

vercel bot commented Feb 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 4:37pm

Comment on lines 89 to 100
const scrollToHashEvent = async () => {
store.jumpToHashPage(hash);
await tick();
let id = hash.slice(1);
const row = document?.querySelector(`[data-eventid="${id}"]`);
if (row) {
setTimeout(() => {
row?.scrollIntoView({ behavior: 'smooth' });
}, 500);
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how I feel about this logic being in a holocene component and enabled by default. Was there a reason you didn't add this to the parent wherever it was needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was on the fence about that too. I could expose jumpToHash to the slot but then you would need to run the scrollToHashEvent function on every row, which doesn't work if the row isn't rendered (i.e. on the same page you are looking at).

I do think we'll want this eventually for other paginated tables but agreed that this isn't ideal in that it's for events only. I added a hashField prop so that it only runs if that's provided and then you can pass whatever data-{hashField} attribute you want to scroll

@Alex-Tideman Alex-Tideman merged commit 7a49763 into main Feb 24, 2025
15 checks passed
@Alex-Tideman Alex-Tideman deleted the scroll-to-event-id branch February 24, 2025 16:46
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