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.

Sign up for GitHub

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

Jump to bottom

Deploying ScrollArea #10423

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Deploying ScrollArea #10423

wants to merge 19 commits into from
+126 −109

Conversation

Duncid
Copy link
Contributor

@Duncid Duncid commented Jan 31, 2025 •
edited
Loading

Description

Sparkle

  • Deploying sparkle with updated scrollArea
  • Updated Sidebar look & feel
  • Cleaned Feedback look & feel

Tests

  • Assistant detail view
  • Assistant Builder perf view
  • Sidebar Conversation

Risk

low

Deploy Plan

merge

@Duncid Duncid requested a review from JulesBelveze January 31, 2025 10:37
JulesBelveze
JulesBelveze reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@JulesBelveze JulesBelveze left a comment

Choose a reason for hiding this comment

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

As a general comment, could we maybe set the scrolling to active when a user is scrolling and set it to inactive after a short delay?

Note:

  • you have a huge diff on the front package-lock.json, is that wanted?
  • could we split the PR in two? To have front changes in one and sparkle changes in the other

@Duncid
Copy link
Contributor Author

Duncid commented Feb 4, 2025

@JulesBelveze cleaned the diff of package-lock!

As a general comment, could we maybe set the scrolling to active when a user is scrolling and set it to inactive after a short delay?

Not sure what you mean here. What would you like to achieve with this?

could we split the PR in two? To have front changes in one and sparkle changes in the other

There are no breaking changes in the sparkle changes, so I think we can just merge and update right after

@JulesBelveze
Copy link
Contributor

Basically having the state set to active only when the user is scrolling

There are no breaking changes in the sparkle changes, so I think we can just merge and update right after

Definitely no breaking changes, but the front changes are not related to Creating a new active state for ScrollAre. But no big deal.

@Duncid Duncid changed the title Creating a new active state for ScrollArea Deploying ScrollArea Feb 5, 2025
Duncid and others added 2 commits February 4, 2025 16:26
@Duncid
Cleaning Sparkle focusing on deploy
be4a187
@JulesBelveze
[front/assistant_builder] - refactor: simplify loading spinner contai…
08f54d0 
…ner in FeedbacksSection

 - Remove unnecessary fragment wrapper around loading spinner component
JulesBelveze
JulesBelveze approved these changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@JulesBelveze JulesBelveze left a comment

Choose a reason for hiding this comment

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

LGTM 👌🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JulesBelveze JulesBelveze JulesBelveze approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Duncid @JulesBelveze