fix(frontend): prevent page shift when area it is scrollable

[Guilhem-lm]

Important

Adds a Scroll component to handle scrollable areas, preventing layout shifts, and updates multiple pages to use this feature.

• Behavior:
  • Introduces Scroll component in Scroll.svelte to handle scrollable areas, preventing layout shifts when scrollbars appear.
  • Adds useScroll prop to CenteredPage.svelte to conditionally wrap content in Scroll component.
  • Updates pages like folders/+page.svelte, groups/+page.svelte, and kafka_triggers/+page.svelte to use useScroll prop.
• Components:
  • Scroll.svelte: New component with overflow-y-auto and scrollbar-gutter: stable both-edges to stabilize layout.
  • CenteredPage.svelte: Modified to include useScroll prop, wrapping content in Scroll if true.
• Layout:
  • Introduces layout-context.ts to manage layout-related state, such as topBarHeight.
  • Updates +layout.svelte to track and update topBarHeight for consistent layout management.
• Misc:
  • Minor updates to several +page.svelte files to integrate useScroll prop, ensuring consistent scroll behavior across pages.

^{This description was created by for dbdb166. It will automatically update as commits are pushed.}

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	97406f2
Status:	✅  Deploy successful!
Preview URL:	https://83717144.windmill.pages.dev
Branch Preview URL:	https://glm-use-melt-scroll-area.windmill.pages.dev

View logs

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to dbdb166 in 2 minutes and 23 seconds

More details

• Looked at 377 lines of code in 19 files
• Skipped 0 files when reviewing.
• Skipped posting 26 drafted comments based on config settings.

1. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:568

• Draft comment:
  Duplicate declaration of 'deployUiSettings'. It is already declared at an earlier line. Remove or consolidate to avoid shadowing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:429

• Draft comment:
  Consider wrapping asynchronous calls in loadSettings with try/catch for better error handling to avoid unhandled promise rejections.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:711

• Draft comment:
  The reactive assignment updating URL query parameters (line 711) could lead to repeated navigations. Consider debouncing or optimizing updates to avoid performance issues during rapid tab changes.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

4. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:581

• Draft comment:
  Reactive statement using $workspaceStore && loadSettings() may trigger repeated API calls on every store change. Consider using onMount or debounced subscriptions to reduce unnecessary reloads.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:568

• Draft comment:
  The variable 'deployUiSettings' is declared with different types in the same component. Use distinct names for different contexts (e.g., one for deploy UI settings and another for Git sync settings) to avoid type conflicts and improve clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. frontend/src/routes/(root)/(logged)/routes/+page.svelte:290

• Draft comment:
  Inline async event handlers (e.g. in on:click attributes) reduce readability and make error handling less clear. Consider moving these handlers into separate functions.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. frontend/src/routes/(root)/(logged)/routes/+page.svelte:290

• Draft comment:
  Ensure that href attributes constructed with string interpolation using backticks and Svelte syntax are correctly formed to avoid runtime errors.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. Multiple files (triggers pages, etc.):90

• Draft comment:
  Directly using 'err.body' in error handling (e.g., sendUserToast) might expose sensitive error details. Sanitize error messages before displaying them.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. frontend/src/routes/(root)/(logged)/workers/+page.svelte:114

• Draft comment:
  Intervals for updating data (e.g., worker pings) are properly cleared in onDestroy, which is good. Consider reviewing the frequency of these API calls for performance optimization.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. Multiple files:1

• Draft comment:
  Repeatedly accessing store values with '$workspaceStore' and '$userStore' throughout reactive statements can clutter the code. Consider destructuring these store values into local variables once to improve readability and possibly performance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. Various files with API calls in catch blocks:100

• Draft comment:
  When accessing error objects (e.g. 'err.body'), ensure the error structure is validated to avoid unexpected undefined values and to prevent leaking potential sensitive details. Consider adding checks or sanitization.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. frontend/src/routes/(root)/(logged)/+page.svelte:312

• Draft comment:
  There appears to be a typographical error in the PickHubApp block. The closing Button tag is split and has an extra '>' on its own line (line 313). It should be corrected to properly close the Button tag (e.g., '') in a single, coherent tag.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. frontend/src/routes/(root)/(logged)/folders/+page.svelte:91

• Draft comment:
  Typographical error: Consider changing 'homogenous RBAC permissions' to 'homogeneous RBAC permissions' in the tooltip for better clarity.
• Reason this comment was not posted:
  Comment was on unchanged code.

14. frontend/src/routes/(root)/(logged)/folders/+page.svelte:190

• Draft comment:
  Typographical suggestion: Consider revising the deletion option text from ' (require owner permissions)' to ' (requires owner permissions)' for grammatical consistency.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

15. frontend/src/routes/(root)/(logged)/folders/+page.svelte:91

• Draft comment:
  Typographical improvement: For consistency with the table headers, consider changing 'schedule' to 'schedules' in the tooltip text.
• Reason this comment was not posted:
  Comment was on unchanged code.

16. frontend/src/routes/(root)/(logged)/groups/+page.svelte:87

• Draft comment:
  Typo: The tooltip text currently reads "homegenous permissions". Consider changing it to "homogeneous permissions".
• Reason this comment was not posted:
  Comment was on unchanged code.

17. frontend/src/routes/(root)/(logged)/groups/+page.svelte:204

• Draft comment:
  Typo: The tooltip for Instance Groups reads "...are groups shared by every workspaces". It would be clearer to change it to "...are groups shared by every workspace" or "...are groups shared by all workspaces".
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

18. frontend/src/routes/(root)/(logged)/mqtt_triggers/+page.svelte:134

• Draft comment:
  Consider renaming 'filterItemsPathsBaseOnUserFilters' to 'filterItemsPathsBasedOnUserFilters' for improved clarity and consistency.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

19. frontend/src/routes/(root)/(logged)/nats_triggers/+page.svelte:133

• Draft comment:
  Typo: Consider renaming the function 'filterItemsPathsBaseOnUserFilters' to 'filterItemsPathsBasedOnUserFilters' for proper spelling and clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

20. frontend/src/routes/(root)/(logged)/schedules/+page.svelte:164

• Draft comment:
  Typo in function name: consider renaming filterItemsPathsBaseOnUserFilters to filterItemsPathsBasedOnUserFilters to correctly use 'based on'.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

21. frontend/src/routes/(root)/(logged)/sqs_triggers/+page.svelte:133

• Draft comment:
  Consider renaming the function 'filterItemsPathsBaseOnUserFilters' to 'filterItemsPathsBasedOnUserFilters' to improve clarity and correct the phrasing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

22. frontend/src/routes/(root)/(logged)/sqs_triggers/+page.svelte:277

• Draft comment:
  Consider capitalizing 'sqs' to 'SQS' in the message 'No sqs triggers' for consistency with other parts of the UI.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

23. frontend/src/routes/(root)/(logged)/variables/+page.svelte:327

• Draft comment:
  Typo: In the message for expired access_token, it says "or you can request is to be refreshed". Consider changing it to "or you can request it to be refreshed".
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

24. frontend/src/routes/(root)/(logged)/variables/+page.svelte:202

• Draft comment:
  Typo: In the Tooltip description, it says "Contextual variables are passed as environment variables when running a script and depends on the execution context." Since 'contextual variables' is plural, it should be "depend on the execution context."
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

25. frontend/src/routes/(root)/(logged)/workers/+page.svelte:400

• Draft comment:
  Typo: In the tooltip text, change 'every workers' to 'every worker' for better grammar.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

26. frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte:1279

• Draft comment:
  Typographical error: In the tooltip text for S3 resource details, the phrase 'access the to entire content' is used. It should probably be 'access to the entire content' instead.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_nOrFxb2gOLm22cd9

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.