-
-
Notifications
You must be signed in to change notification settings - Fork 236
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 store scroll feature to BitAppShell (#10233) #10234
Add store scroll feature to BitAppShell (#10233) #10234
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes add a mechanism to manage scroll positions using session storage. A new static property Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant AS as AppShell
participant SS as Session Storage
UI->>AS: onScroll(event)
AS->>AS: storeScroll(url, scrollValue)
AS->>SS: Save updated scroll position
Note over AS,SS: Scroll state stored in session
UI->>AS: initScroll(url)
AS->>SS: Retrieve stored scroll position
SS-->>AS: Return scroll position
AS->>UI: Apply scroll position
UI->>AS: afterRenderScroll()
AS->>AS: storeScroll(url, updatedValue)
AS->>SS: Update stored scroll position
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShell.ts
(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShell.ts
[error] 14-14: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 51-51: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (4)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShell.ts (4)
3-4
: Good addition of a constant for session storage key.Adding a named constant for the session storage key improves code maintainability and readability, making it easier to reference and update if needed in the future.
28-28
: Improved scroll management with centralized logic.Using the
storeScroll
method here is a good improvement that centralizes the scroll position management logic.
46-47
: Improved scroll event handling with centralized storage.Using the
storeScroll
method here simplifies the event handler and ensures consistent storage behavior across the application.
15-16
: Verify null check implementation in conditional.The exclamation mark in
AppShell._scrolls[url]!
is a non-null assertion operator that tells TypeScript to treat the value as non-null. Ensure this doesn't cause runtime errors if the value is actually undefined.Verify that
AppShell._scrolls[url]
is always defined before this line executes, or consider a safer alternative:- if (AppShell._scrolls[url]! > 0) { + if (AppShell._scrolls[url] && AppShell._scrolls[url] > 0) {
closes #10233
Summary by CodeRabbit