bug: milestone alignment - migrate rendering to d3

[SgtPooki]

Milestone rendering is inaccurate and difficult to debug and get accurate because of the use of css-grids.

While this issue is intended to track the migration to something more accurate, here are some sub-items that have popped up due to the use of css-grid:

• bug: milestone alignment is off #236
• "Today" marker inaccurate in some views #189

Starmaps still impacted by inaccurate css-grid rendering:

• https://www.starmap.site/roadmap/github.com/drand/roadmap/issues/1
  • 2023-06-30 date is not ending on Jul 1 header
  • two milestones with 2023-09-30 are not ending on oct 1 header
• https://www.starmap.site/roadmap/github.com/cryptonetlab/roadmap/issues/22
  • 2023-06-30 date is not ending on Jul 1 header
  • 2023-09-30 are not ending on oct 1 header
    • I believe a 2023-12-31 item whose "gridCol" doesn't fit within displayed timeRange is pushing them back

Tasks

Beta Give feedback

1. feat: d3-migration - Match existing header (tabs) and breadcrumbs
2. feat: d3-migration - Match date labels and ticks
3. feat: d3-migration - implement collision detection on milestone cards so they fold up
4. feat: d3-migration - roadmap width
5. feat: d3-migration - update milestone appearance
6. feat: d3-migration - update milestone appearance - github icon
7. feat: d3-migration - update milestone appearance - progress bar
8. feat: d3-migration - update milestone appearance - text polish
9. feat: d3-migration - roadmap container - ensure milestone items with large Y increase container height
10. feat: d3-migration - panning - limit panning to min/max based on visible milestones
11. feat: d3-migration - header - date labels and ticks - ensure no text overlap
12. feat: d3-migration - header - date labels and ticks - ensure smart text&column updates when at zoom extremes
13. feat: d3-migration - Implement detailed view
14. fix: d3-migration - zoom/pan event handlers break when swapping between tabs
15. feat: d3-migration - today marker style updates
16. fix: d3-migration - default zoom level should be more intuitive
17. fix: d3-migration - better handling of tall roadmaps (vertical scrolling, header pinning, etc)
18. feat: d3-migration - create "legacy" branch and deploy legacy.starmap.site where users can go to for old functionality #364
    +0
    
19. fix: d3-migration - unit tests
20. fix: d3-migration - e2e tests
21. fix: d3-migration - default view finding for ipfs/roadmap#102 #369
    +0
    
22. chore: d3-migration - remove any unused code & files
23. feat: d3-migration - add link to legacy.starmap.site in header
24. fix: d3-migration - breadcrumb navigation #371
    +0
    
25. fix: d3-migration - track down what's not loading for Molly #374
    +0

[SgtPooki]

Getting the header positioning down:

[SgtPooki]

dialing in on new mocks for grid header items:

[SgtPooki]

Pausing this in favor of ipfs/ipfs-webui#2079

[SgtPooki]

I pushed the code to the linked branch, here is the current status of the UI with d3:

[AlexxNica]

I should've done a better job explaining the decision to use HTML and CSS instead of D3 for this project. (I'll transfer decisions made in DMs and Google Docs to GitHub in the next few days for better clarity)

Here's a bit of context behind the decision (my message from a conversation with @wilkyr31d regarding implementation details):

I didn't find any libraries to use for the renderer that wasn't overly complex, so I had to build one from scratch.

Almost all solutions for timeline-like things are using D3 and SVG/vector directly. The not-so-great thing about using D3/SVG/vector directly is that it doesn't have the usability/flexibility of HTML. Those libraries are great if all we're going to do is render a timeline and forget about it. But since we require a lot of interactivity from the user (rendering, clicking, event-related states), HTML and CSS would the best solution as far as I'm aware without the increased complexity of messing with vectors and path/positioning calculations for every element–which I don't have enough experience to make happen in a short time for an MVP.

This is specific to timelines, though. For rendering complex graph-like, connected things, there are great libraries out there. Since we need a timeline, those libraries don't fit our use case.

I still believe D3 isn't the right tool for the job here. It adds too much complexity and increases the maintenance cost for very little upside. FWIW, GitHub's new roadmap feature uses CSS flexbox with a structure similar to CSS grid and our implementation: https://github.com/orgs/nikas-org/projects/13/views/5.

The grid layout also makes it easy to implement features like the list view (#120) using placement and alignment features: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Grid_Layout/Basic_Concepts_of_Grid_Layout

Here's a screenshot with some quick modifications to the CSS for the list view (I'll add more details on #120):

[SgtPooki]

d3 migration progress

Branch: https://github.com/pln-planning-tools/Starmap/tree/237-enhancement-bug-migrate-rendering-to-d3

Updates:

• panning + zooming support
  • Panning -> click + drag -- naive limitation of either zoom/drag only; not both at the same time (intentional)
  • Zooming -> scrollbar/trackbar
• Today marker is accurate with panning + zooming
• milestone collision detection handled with single-pass binPacking algorithm on "colliding X-range columns"

TODO:

Tasklist in this issue's description will be updated, but things to come are:

1. d3 roadmap width (currently not aligned with the rest of the layout; should be a quick & easy fix)
2. detailed view using d3
3. today marker style updates
4. milestone view polish/updates
   • Add github icon
   • Add progress bar
   • Cap milestone title length - Text rendering when there are longer titles causes issues (see http://localhost:3000/roadmap/github.com/protocol/engres/issues/8?mode=d3)
   • Height of d3 roadmap container does not expand if milestone items are too tall for view (see http://localhost:3000/roadmap/github.com/protocol/engres/issues/8?mode=d3)
5. Zoom / panning polish
   • Fix column text overlapping on various window sizes - Current zoom limit not smart enough to know when (# of header items * width per column) results in text overlap. This currently only happens on very small window sizes when significantly zoomed out.
   • Remove zoom-out limit -> Zooming out is currently limited to a hardcoded value that prevents axis column text (MMM YYYY) from overlapping. Instead, we should dynamically update the text representation and column sizes so that zooming out far starts showing Quarters, years, etc..

Demo video

starmap-d3-progress-1.mp4

[SgtPooki]

milestone appearance is coming along nicely:

[SgtPooki]

progress bars for d3 renderer are in

[SgtPooki]

Done with MOST of the checklist.. except for the most difficult item: detailed view rendering. It shouldn't be too hard, but I wanted to flesh out a really nice simple view first.

Check it out:

starmap-d3-progress-2.mp4

Note: Some roadmaps cause issues, and I will be looking into them. (updated the task list)

[SgtPooki]

I pushed up a PR #358 so users can try out the demo and give feedback..

You can try out the d3 rendering preview at https://starmaps-git-237-enhancement-bug-migrate-ren-3c536e-ipfs-ignite.vercel.app/roadmap/github.com/ipfs/ipfs-gui/issues/106

Things i’m looking for feedback on:

• require CMD/CTRL key for scrolling/panning methods?
• swap pan/zoom directions? what is your intuition telling you?
• header tick/date label transitioning on zoom, too soon? too late? customize-able?
• text sizes, anything else visual…?
• should we have more loose/tight zoom/pan limits?

Most of these things are easy to change, but I'd rather get feedback from users early than late!

[SgtPooki]

Some feedback in slack:

• It's unexpected that horizontal scroll does zooming
• Zooming using my track pad's zoom gesture works really well
• Therefore I'm wondering, why not use scroll for scrolling and zoom for zooming, for those who use a track pad? And offer alternatives for pure mouse users (+/- buttons or a slider)?
• Zooming using my track pad's scroll gesture feels jittery
• Header works well during zooming, I don't see why you'd need to make it customizable
• Everything else looks great!

[AlexxNica]

Hey, just want to drop a note here and say that I feel totally ignored, and if the desire is for this project to be open source and collaborative, there needs to be better – well... – collaboration

I raised some flags on changing the implementation to D3 both here (comment above) and in Slack, and while we had some more interactions in Slack, we didn't reach a conclusion on it. I'm not saying every decision has to go through the community or open-source contributors, but it'd be nice to have a comment stating the decisions you (or the owner/maintainer of the project, which isn't clear) make, especially when there was prior discussion around a change.

I know it's way easier to get things moving without external collaboration, but if that's the current status, I think it needs to be stated somewhere so open-source contributors know when to jump in and help develop the project instead of wasting time and bothering maintainers.

Again, just dropping this comment to collaborate with the project and its future so we can all see it grow as a healthy, open source project that receives contributions from everyone.

[SgtPooki]

@AlexxNica, I'm sorry to hear that you feel ignored. I understand how frustrating that can be. I've been posting updates to this issue to be open and collaborative but I missed an opportunity to announce that I was "officially" starting this work.

This issue was never closed, which means the primary maintainers (IPFS Ignite - IPFS GUI & Tools team) felt that this issue was still in the realm of things to be done. It was recently decided that this work needs to be prioritized, so it was decided (without announcing; apologies) that I would start work on this last week.

I am writing a separate document explaining why we're doing this and will share it in this issue when done. In short, the maintenance of the existing code could be better for the current maintainers, while achieving a much more accurate visual representation for the users. I understand that you feel differently, but the fact is that the two engineers currently responsible for this code feel this way. I understand that you may have some feedback as a user, and I definitely want to hear your thoughts. However, I do want to clarify that the implementation itself is not up for debate.

Open-source collaboration does not mean we need to agree. We encourage you to provide your feedback on the open PR (#358) so that we can make Starmap better for everyone. We expect to have this work completed within the next week, and your input is valuable and will help us create a better product.

[AlexxNica]

@SgtPooki thanks for clarifying! 🙌

[BigLep]

Cool! I took a quick look at the demo. A couple of comments:

1. We are using a date format like "23-Jan-2023". Can we do ISO8601 as we require in the issues themselves (e.g., 2023-01-23)?
2. When I zoom out (see below), items don't order I would expect. I think we want to sort by endDate ASC, startDate ASC, childOrder ASC. Basically, whatever is completing first should be up first. Where there is a tie, look at start date, and where there is still a tie, use the ordering of in the children list.
   • Maybe even better here is to let this be choosable by the user, where they can decide whether to order by startDate, endDate, or children list order.

3. Stepping back, I'm wondering whether as a simplifier, do we even need to binpack? What if we do one milestone per row? Does that make things simpler? I can see how binpacking makes things more compact on the screen, but it then more me it calls into question, "Why are two items on the same row? Do they have a certain relationship?" One item per row removes ambiguity.
   • Here again maybe having a configuration knob could be useful for turning on bin packing or not and we see which option people reach for.

[SgtPooki]

We are using a date format like "23-Jan-2023". Can we do ISO8601 as we require in the issues themselves (e.g., 2023-01-23)?

This was previously changed to be more "human readable," but I can switch to YYYY-MM-DD format.

When I zoom out (see below), items don't order I would expect. I think we want to sort by endDate ASC, startDate ASC, childOrder ASC. Basically, whatever is completing first should be up first. Where there is a tie, look at start date, and where there is still a tie, use the ordering of in the children list.

• Maybe even better here is to let this be choosable by the user, where they can decide whether to order by startDate, endDate, or children list order.

I have heard from some users that leaving the order provided in the issue is desired, but after looking at the roadmap a lot in the past week, I personally would prefer sorting in a way more like what you describe; Ideally, this would be configurable by the user.

By preferring the provided children order (the order provided in the github issue) users should be able to control the order, without us having to maintain any complicated settings/configuration logic. This gives users the most control and us the least amount of work required to support that control.

Also, if we were to order things by dates so that the leftest-X valued milestones are always more "top and left" than comparable milestones, I imagine we would frequently end up with a stair-stepping, non-collapsible view. If users want that view, they should be able to get it with the current logic.

Stepping back, I'm wondering whether as a simplifier, do we even need to binpack?

Compressing the items into less vertical space was an explicit request from Molly during IPFS Thing: "I need to compress the items more; they take up too much vertical space." I've heard similar from other users and have frequently felt the same.

What if we do one milestone per row? Does that make things simpler?

I believe it would complicate things much more significantly. The bin-packing logic is done and should be ready to use in more complicated roadmaps. (still to be confirmed and user-tested with Molly). If we add rows, we need to decide on what semantic meaning that actually has, and how users can specify that meaning with the current configuration options available to them (github issue markdown only)

In a Starmap roadmap, I don't think we really have a concept of rows currently, other than in the detailed view, which is not necessarily a row but a grouping of multiple simple views for all the children of the root issue being viewed.

I could see a future where we do consider the concept of rows/tracks via labels or markdown key/val or something, but that currently doesn't exist in Starmap that I'm aware of. Maybe we can describe more in the user guide about how the only relationship between items is the ETA and how they relate to other ETAs.

Here again maybe having a configuration knob could be useful for turning on bin packing or not and we see which option people reach for.

I believe mollys specific request was to have a "screenshot mode" button that would compress items, so maybe I need to just add a toggle that increases/decreases ySpacing in the binPacking function. This is a very easy logic change, but where does the knob go without getting in the way? (related: #166)

[BigLep]

This was previously changed to be more "human readable," but I can switch to YYYY-MM-DD format.

ISO format is good because there is no ambiguity across international lines. If you are wanting to do a more english format, then I think "Jan 23, 2023" rather than "23 Jan, 2023" (I'm not familiar where that format is common, and it's certainly not the norm in North America).

Ordering

I agree that issue-specified ordering is the best choice if not making this configurable as at least then the user is in control.

What if we do one milestone per row? Does that make things simpler?

I wasn't suggesting to make "rows" a first class primitive. I agree that is more work. I was more thinking that if the children tasks are X, Y, Z then X, Y, and Z are on the own row (no bin-packing). (This is analogous to Github and Notion's timeline view).

This is a very easy logic change, but where does the knob go without getting in the way?

To be scrappy, maybe just make it a query param for now and document it in the readme as an experimental feature? (It'll need to be a query param anyway in the future so that URLs are bookmarkable).

[juliangruber]

Feedback notes:

The default zoom level is unexpected, I would prefer to get ~2 quarters of time?

Zooming and navigating (using a trackpad) works really well!

When you zoom out far and have many overlapping items, it can be necessary to scroll vertically. That doesn't work for me when my pointer is over the roadmap, I have to move it to the side of the page to scroll.

[AlexxNica]

I noticed that the alignment might be a bit off. From the screenshot below, the ETA is set for December 1st but the card isn't in the December box:

[whizzzkid]

@AlexxNica, since there is no concept of time (, or start date for that matter), I would like to imagine, something due on Dec 1, comes due at 12am Dec 1. I think there is a difference in expectations. The calculations can be trivial to fix, if we can fix the expectations.

IMHO this is way better in aligning milestones at the right position, which was not the case with CSS grid, I remember spending a lot of time in one of the older PRs to make issues align right with the grid.

[SgtPooki]

Thanks a ton for the helpful feedback @juliangruber!

The default zoom level is unexpected, I would prefer to get ~2 quarters of time?

Definitely. I thought of this but dropped the ball. I added an action item. I've been thinking of making the zoom level share-able (url param) to help with debugging and collaboration as well. Ideally, any "view" should be re-renderable by just sharing the URL

Zooming and navigating (using a trackpad) works really well!

Yay! There's still some polish on zoom/pan sync that could be done but I'm glad it's at a good space for you, i spent a day last week polishing this up

When you zoom out far and have many overlapping items, it can be necessary to scroll vertically. That doesn't work for me when my pointer is over the roadmap, I have to move it to the side of the page to scroll.

I have been debating how to handle this. I think we would want the header to stay locked in place, and allow users to scroll vertically within the view. There's also the problem of very tall views and the need to screenshot the entirety of the roadmap, so we want to be able to allow scrolling, while also allowing users to see the entire roadmap if they want. I'll add an action item to address this.

[BigLep]

Ideally, any "view" should be re-renderable by just sharing the URL

Definitely like this as a tenet to follow. (Obviously the thing we can control is someone's window size and user-agent, but this will still be great.)

[BigLep]

From the screenshot below, the ETA is set for December 1st but the card isn't in the December box

Certainly a minor issue that I don't think we need to prioritize, but I think a good rule of thumb is that the time we use is the very end of a given day. The reason I suggest that is because folks will naturally say, "this will be done by end of quarter 2023-06-30". In that case we should fill up through June.

But again, not critical here.

[AlexxNica]

@AlexxNica, since there is no concept of time (, or start date for that matter), I would like to imagine, something due on Dec 1, comes due at 12am Dec 1. I think there is a difference in expectations. The calculations can be trivial to fix, if we can fix the expectations.

IMHO this is way better in aligning milestones at the right position, which was not the case with CSS grid, I remember spending a lot of time in one of the older PRs to make issues align right with the grid.

@whizzzkid Makes sense. (though I agree with @BigLep - I believe the most common understanding of due dates/ETAs refers to the end of that specific day - or within business hours in case of a product release, etc.)

Clarifying that last comment: it wasn't meant to suggest using css grids, it was just some feedback in case that was a bug 😄

[SgtPooki]

The latest changes update all milestone card ETAs to be "endof('day')"

[SgtPooki]

now with default view finding: Try out

https://starmaps-git-237-enhancement-bug-migrate-ren-3c536e-ipfs-ignite.vercel.app/roadmap/github.com/ipfs/ipfs-gui/issues/106

Now with linkable zoomconfig:

Check out https://starmaps-git-237-enhancement-bug-migrate-ren-3c536e-ipfs-ignite.vercel.app/roadmap/github.com/ipfs/ipfs-gui/issues/106#d3x=799.10&d3y=198.90&d3k=0.20

Note: If you are going to pan, use either drag or horizontal swipe (trackpad). Both will end up causing jitter for some reason. I looked into it a lot today but spent too much time on it. I'm going to disable horizontal scrolling for panning if anyone has issues with the current functionality.

[whizzzkid]

@SgtPooki looks like the fetch issues for stale data is still problematic. Cleaning the storage makes it go away.

Screen.Recording.2023-05-10.at.1.16.52.AM.mov

[SgtPooki]

@SgtPooki looks like the fetch issues for stale data is still problematic. Cleaning the storage makes it go away.

I'm hoping #363 will help us dive into those more

[SgtPooki]

it appears like #363 fixed the fetch errors. I bet it was the cachedResponse=undefined that was causing the failures.

[SgtPooki]

FYI: #358 has approval, and will be merged this week unless there are pressing concerns users have regarding the demo you can visit at https://starmaps-git-237-enhancement-bug-migrate-ren-3c536e-ipfs-ignite.vercel.app/