Skip to content

feat(assistant): add component#2603

Open
RemiBonnet wants to merge 11 commits intonew-navigationfrom
feat/new-nav/documentation
Open

feat(assistant): add component#2603
RemiBonnet wants to merge 11 commits intonew-navigationfrom
feat/new-nav/documentation

Conversation

@RemiBonnet
Copy link
Copy Markdown
Member

@RemiBonnet RemiBonnet commented Apr 16, 2026

Summary

Add assistant everywhere

Screenshots / Recordings

https://www.loom.com/share/2efe2e4bf9af43c88823d06ec43ea1e7

Testing

  • Changes tested locally in the relevant Console's pages and Storybooks
  • yarn test or yarn test -u (if you need to regenerate snapshots)
  • yarn format
  • yarn lint

PR Checklist

  • I followed naming, styling, and TypeScript rules (see .cursor/rules)
  • I performed a self-review (diff inspected, dead code removed)
  • I titled the PR using Conventional Commits with a scope when possible (e.g. feat(service): add new Terraform service) - required for semantic-release
  • I only kept necessary comments, written in English (watch for useless AI comments)
  • I involved a designer to validate UI changes if I am not a designer
  • I covered new business logic with tests (unit)
  • I confirmed CI is green (Codecov red can be accepted)
  • I reviewed and executed locally any AI-assisted code

@RemiBonnet RemiBonnet marked this pull request as ready for review April 16, 2026 10:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 5.00000% with 38 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (new-navigation@6a865a6). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...eature/src/lib/assistant-panel/assistant-panel.tsx 0.00% 18 Missing ⚠️
...re/src/lib/assistant-context/assistant-context.tsx 0.00% 11 Missing ⚠️
.../assistant-panel-outlet/assistant-panel-outlet.tsx 0.00% 4 Missing ⚠️
...ssistant/feature/src/lib/dot-status/dot-status.tsx 0.00% 3 Missing ⚠️
...re/src/lib/assistant-trigger/assistant-trigger.tsx 0.00% 1 Missing ⚠️
.../assistant/feature/src/lib/need-help/need-help.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##             new-navigation    #2603   +/-   ##
=================================================
  Coverage                  ?   44.92%           
=================================================
  Files                     ?      743           
  Lines                     ?    17645           
  Branches                  ?     5184           
=================================================
  Hits                      ?     7927           
  Misses                    ?     8328           
  Partials                  ?     1390           
Flag Coverage Δ
unittests 44.92% <5.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TheoGrandin74
Copy link
Copy Markdown
Contributor

LGTM! Only small feedback is that the transition when the height change on scroll is different than the header so we have weird jump when scrolling like here:

Enregistrement.de.l.ecran.2026-04-16.a.13.52.35.mov

@RemiBonnet
Copy link
Copy Markdown
Member Author

RemiBonnet commented Apr 16, 2026

LGTM! Only small feedback is that the transition when the height change on scroll is different than the header so we have weird jump when scrolling like here:

Thanks, I just fixed this! I tightened the top and height transition so the panel tracks the header more closely while scrolling and feels snappier

It still can’t be a perfect sticky behavior on our side because the panel has to recalculate both top and height as the breadcrumb/header changes, so there can be a small transition instead of a fully native sticky feel

@TheoGrandin74
Copy link
Copy Markdown
Contributor

Looks better transition wise 👍

We can still have some wonky behavior if you stop the scroll at the wrong time, not sure if we can do something about it (minor)
image

Also feels like maybe the height is maybe miscalculated by a pixel because once we scroll the border disappear above the helper
image

@@ -0,0 +1,41 @@
// XXX: Use this store to avoid re-rendering all pages or routes just to open the documentation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please explain the re-rendering issue you're talking about here? 🙏 Because for now, it is not clear to me what this workaround is actually solving

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep, the issue was mostly about where the open state lived. When it was a useState higher up in the app, opening/closing the documentation panel was causing the whole page tree to re-render, not just the assistant. That created small visual glitches in a few places, especially image-based icons that would flicker/remount

The store workaround is just to scope that state so only the assistant-related components subscribe to it, which avoids those unnecessary re-renders

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. That still sounds a bit hacky to me. Couldn't we use a Context instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was actually a Context + useState at first, and that’s what caused the issue. Since it lived too high in the tree, opening/closing the panel was still re-rendering a much larger part of the app than needed

I’ll try moving it lower on Monday instead of keeping it around the router level, that might be enough to solve it cleanly

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I’ve moved the open state inside the router so it no longer re-renders the whole page, it should behave better now!

3aa60ba

@RemiBonnet
Copy link
Copy Markdown
Member Author

RemiBonnet commented Apr 16, 2026

We can still have some wonky behavior if you stop the scroll at the wrong time, not sure if we can do something about it (minor)

Should be better now, I tried moving it directly to CSS to improve the sticky behavior, but there are still a few edge cases. I can take another look on Monday if I come up with a better idea to fix this small jump

Also feels like maybe the height is maybe miscalculated by a pixel because once we scroll the border disappear above the helper

Thanks, fixed !

@RemiBonnet RemiBonnet requested a review from rmnbrd April 20, 2026 07:52
@RemiBonnet RemiBonnet added the V5 label Apr 20, 2026
@RemiBonnet RemiBonnet force-pushed the feat/new-nav/documentation branch from 3aa60ba to 7c30708 Compare April 21, 2026 07:16
@rmnbrd
Copy link
Copy Markdown
Contributor

rmnbrd commented Apr 21, 2026

Depending on the scroll amount, you can end up with something like this:

Screenshot 2026-04-21 at 15 53 37

@RemiBonnet RemiBonnet force-pushed the feat/new-nav/documentation branch from 7c30708 to f240fee Compare April 21, 2026 14:22
@RemiBonnet RemiBonnet force-pushed the feat/new-nav/documentation branch from 7932b33 to 32cc002 Compare April 21, 2026 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants