-
-
Notifications
You must be signed in to change notification settings - Fork 3k
refactor: convert banners and psas to solidjs (@miodec) #7395
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
base: master
Are you sure you want to change the base?
Conversation
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
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.
Pull request overview
This PR refactors the banner and PSA (Public Service Announcement) system from a DOM-manipulation-based approach to a reactive SolidJS implementation, improving maintainability and reactivity.
Changes:
- Introduced a new SolidJS store (
banners.ts) to manage banner state reactively - Created a new SolidJS component (
Banners.tsx) for rendering banners - Migrated all
Notifications.addPSA()andNotifications.addBanner()calls to use the newaddBanner()function from the store - Removed the banner-event observable system in favor of SolidJS reactivity
- Removed banner/PSA-specific code from the notifications module
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/ts/stores/banners.ts | New SolidJS store for managing banner state with add/remove/get functions |
| frontend/src/ts/components/layout/overlays/Banners.tsx | New SolidJS component for rendering banners with reactive updates |
| frontend/src/ts/hooks/useTailwindBreakpoints.ts | New hook for tracking Tailwind breakpoints (added but not used in this PR) |
| frontend/src/ts/observables/banner-event.ts | Removed - replaced by SolidJS reactivity |
| frontend/src/ts/states/connection.ts | Updated to use new banner store for offline notifications |
| frontend/src/ts/elements/psa.ts | Migrated PSA banners to use new store |
| frontend/src/ts/elements/notifications.ts | Removed banner/PSA functionality, keeping only notification logic |
| frontend/src/ts/elements/merch-banner.ts | Updated to use new banner store |
| frontend/src/ts/controllers/ad-controller.ts | Replaced BannerEvent subscription with SolidJS createEffect |
| frontend/src/ts/components/layout/overlays/Overlays.tsx | Added Banners component to the layout |
| frontend/src/ts/auth.ts | Updated to use new banner store for account name change banner |
| frontend/src/ts/ape/adapters/ts-rest-adapter.ts | Updated to use new banner store for version mismatch banner |
| frontend/src/index.html | Removed bannerCenter div (now rendered by SolidJS) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
|
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
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.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.