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

Improved marks mapping #3142

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

granth23
Copy link
Contributor

@granth23 granth23 commented Jan 23, 2025

Problem

image

Sidebar with Date marks didn't map to the date accurately on the /explore/fresh-releases/ page.

Solution

image

Modified the method of percentage calculation to accommodate a better mapping to the date searched for by the user.

Action

  1. Please review the changes to ensure they align with the project's style guide.
  2. Verify that the updated mapping improvements work as intended on the live site.

@granth23 granth23 marked this pull request as draft January 23, 2025 09:35
@granth23 granth23 marked this pull request as ready for review January 23, 2025 09:35
@granth23 granth23 marked this pull request as draft January 23, 2025 09:39
@granth23 granth23 marked this pull request as ready for review January 23, 2025 09:45
@MonkeyDo
Copy link
Member

MonkeyDo commented Jan 23, 2025

Hehe, great minds think alike !

I've done a similar change in #3138

However that PR also puts in a date whenever a week has passed, I thought it might be useful to avoid having huge gaps in the timeline, but I think it might make it worse in the end. I'll check your solution and see which one works best.

@MonkeyDo
Copy link
Member

MonkeyDo commented Jan 23, 2025

So this is pretty similar results to what I tried myself, and while it improves the timeline I think there are still issues that I don't know how to solve.
Primarily, clicking on a date in the timeline never gets me to the actual date...
I think part of the issue is that we are trying to make our data fit a library that isn't really meant for this (rc-slider).

I think the solution would instead be to roll out our own component where we can control things a bit better.
In particular, we can turn the date separators on the page into headings (<h4 id={myDateStr}> or something like that), and make the dates on the timeline anchor tags (<a href={`#${myDateStr}`}>) and let the browser deal with scrolling you accurately to the desired spot on the page. I tested it manually and it works way nicer, basically for free.

That leaves us with creating the component and styles, and reacting to page scroll/click on the timeline
Those last two points are fairly easy, and I tested that too (scroll event listener + calculating of container height and timeline height + rescaling one to the other).
Considering we already calculate percentages for positioning, overall I think it is pretty achievable.

What do you think about that plan?
And would you be interested in working on this? If so I'll share all my trials in more details.

@granth23
Copy link
Contributor Author

Originally I was thinking with the idea of anchor tags but didn't feel like changing the entire thing but surely I can work on it, do share your trials will get on to it right away.

@granth23
Copy link
Contributor Author

One of the problems with anchor tags is that once the user redirects himself, the scroll will automatically move here and there from the date due to the percentage implementation, any idea on how to resolve that?

@MonkeyDo
Copy link
Member

One of the problems with anchor tags is that once the user redirects himself, the scroll will automatically move here and there from the date due to the percentage implementation, any idea on how to resolve that?

I don't think I understand what you mean by "redirects himself".

Do you mean that there will be a mismatch between the vertical placement of the dates in the timeline and the actual position of the "cursor" (the position indicator) on the timeline?
I think that's a possibility. I suppose we could find a hacky way to get the actual Y position on the page for each date-separator, but what I have in mind won't be very React-y.

@granth23
Copy link
Contributor Author

Yes that's what I mean that the placement won't be the same, do you think we could ignore the threshold part and just make it equally distributed than total releases focused, the only problem with that will be the concept behind which date has more releases.

@MonkeyDo
Copy link
Member

MonkeyDo commented Jan 27, 2025

I think you can go ahead without worrying too much about the placement of dates on the timeline.
We can try equal distribution, or we can try later to implement some other mechanism (I have some ideas but will need to test them).
Either way, it won't be very different from the current broken placement, so we're not regressing in terms of functionality.

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