-
-
Notifications
You must be signed in to change notification settings - Fork 56
refactor: Complete project rewrite with Svelte 5 #339
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: dev
Are you sure you want to change the base?
refactor: Complete project rewrite with Svelte 5 #339
Conversation
…S issues in dev. environment
…l components and pages
…ty and maintainability.
…ing, robust persisted state, new skeleton/error/query UI components, and intersection observer utility. Remove TanStack Query.
…n files to a major file and restored and rewrote features from the old implementation
…2/revanced-website into refactor/rewrite
…ture with upstream repository
.env.example
Outdated
| VITE_RV_API_URL=https://api.revanced.app | ||
| VITE_RV_STATUS_URL=https://status.revanced.app | ||
| VITE_RV_EMAIL=[email protected] | ||
| VITE_GOOGLE_TAG_MANAGER_ID= | ||
| PUBLIC_RV_DMCA_GUID= |
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.
Why was this change made?
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.
It’s just for clarity.
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.
I don't believe it's necessary, VITE holds no relevance to this project
src/lib/api/queries.ts
Outdated
| health: () => ({ | ||
| queryKey: queryKeys.health(), | ||
| queryFn: checkApiHealth, | ||
| staleTime: 30 * 1000, | ||
| gcTime: 60 * 1000, | ||
| refetchInterval: 5 * 60 * 1000 | ||
| }) | ||
| }; |
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.
What is health? there's no such thing in the API
src/lib/stores/apiHealth.svelte.ts
Outdated
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.
Why is there an API health store? And why does it follow an entirely different codestyle than usual?
src/lib/utils/dev.ts
Outdated
| export function dev_warn(part: string, ...args: unknown[]): void { | ||
| if (dev) { | ||
| console.warn(`[${part}]:`, ...args); | ||
| } | ||
| } | ||
|
|
||
| export function dev_error(part: string, ...args: unknown[]): void { | ||
| if (dev) { | ||
| console.error(`[${part}]:`, ...args); | ||
| } | ||
| } |
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.
These are unused, and do we even need this file to begin with?
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.
What is this used for? revanced.app doesn't need this anywhere
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.
It’s used for transitions in the NavBar.
https://github.com/ReVanced/revanced-website/blob/main/src/util/horizontalSlide.js
| .tag-chip { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| gap: 6px; | ||
| height: 32px; | ||
| padding: 0 12px; | ||
| border-radius: 8px; | ||
| border: none; | ||
| background-color: var(--tertiary); | ||
| color: var(--text-four); | ||
| font-size: 0.85rem; | ||
| font-family: inherit; | ||
| letter-spacing: 0.02em; | ||
| cursor: default; | ||
| user-select: none; | ||
| transition: background-color 0.2s var(--bezier-one), | ||
| border-color 0.2s var(--bezier-one), | ||
| color 0.2s var(--bezier-one); | ||
| } | ||
| .tag-chip.clickable { | ||
| background-color: transparent; | ||
| border: 1px solid var(--border); | ||
| cursor: pointer; | ||
| } | ||
| .tag-chip.clickable:hover { | ||
| background-color: var(--surface-three); | ||
| } | ||
| .tag-chip.clickable.selected { | ||
| background-color: var(--tertiary); | ||
| border-color: transparent; | ||
| color: var(--primary); | ||
| } | ||
| .check-icon { | ||
| display: flex; | ||
| align-items: center; | ||
| margin-left: -4px; | ||
| } | ||
| .check-icon :global(svg) { | ||
| width: 16px; | ||
| height: 16px; | ||
| } | ||
| .tag-chip:disabled { | ||
| cursor: default; | ||
| } |
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.
Indent these
| .tooltip-wrapper { | ||
| position: relative; | ||
| display: inline-flex; | ||
| outline: none; | ||
| } | ||
| .tooltip { | ||
| position: absolute; | ||
| bottom: 100%; | ||
| left: 50%; | ||
| transform: translateX(-50%); | ||
| margin-bottom: 8px; | ||
| padding: 0.75rem 1rem; | ||
| background-color: var(--surface-three); | ||
| color: var(--text-four); | ||
| border-radius: 12px; | ||
| font-size: 16px; | ||
| font-weight: 500; | ||
| white-space: nowrap; | ||
| box-shadow: var(--drop-shadow-one); | ||
| opacity: 0; | ||
| visibility: hidden; | ||
| transition: | ||
| opacity 0.2s ease, | ||
| visibility 0.2s ease; | ||
| z-index: var(--z-dropdown); | ||
| pointer-events: none; | ||
| } | ||
| .tooltip::after { | ||
| content: ''; | ||
| position: absolute; | ||
| top: 100%; | ||
| left: 0; | ||
| right: 0; | ||
| height: 12px; | ||
| } | ||
| @media (hover: hover) { | ||
| .tooltip-wrapper:hover .tooltip { | ||
| opacity: 1; | ||
| visibility: visible; | ||
| } | ||
| .tooltip-wrapper:hover .tooltip.interactive { | ||
| pointer-events: auto; | ||
| } | ||
| } | ||
| @media (hover: none) { | ||
| .tooltip-wrapper:focus-within .tooltip { | ||
| opacity: 1; | ||
| visibility: visible; | ||
| } | ||
| .tooltip-wrapper:focus-within .tooltip.interactive { | ||
| pointer-events: auto; | ||
| } | ||
| } | ||
| .tooltip :global(a) { | ||
| color: var(--primary); | ||
| text-decoration: none; | ||
| font-size: 17px; | ||
| } | ||
| .tooltip :global(a:hover) { | ||
| text-decoration: underline; | ||
| } | ||
| .tooltip :global(p) { | ||
| margin: 0 0 0.25rem 0; | ||
| font-size: 17px; | ||
| color: var(--text-four); | ||
| opacity: 0.8; | ||
| } |
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.
Indent these
| .contributor:hover { | ||
| background: var(--surface-three); | ||
| } | ||
| h5 { | ||
| .contributor:hover .name { | ||
| text-decoration: underline var(--primary); | ||
| color: var(--text-one); | ||
| } |
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.
Indent these
| .wallets { | ||
| width: clamp(200px, 75vw, 375px); | ||
| } | ||
| .wallet { | ||
| width: 100%; | ||
| font-size: 0.9rem; | ||
| font-family: inherit; | ||
| background-color: var(--surface-seven); | ||
| border: none; | ||
| color: var(--text-four); | ||
| cursor: pointer; | ||
| text-align: left; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| padding: 0.75rem 1.25rem; | ||
| transition: filter 0.4s var(--bezier-one); | ||
| } | ||
| .wallet:hover { | ||
| filter: brightness(85%); | ||
| } | ||
| .wallet-name { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 0.5rem; | ||
| } | ||
| .wallet-name img { | ||
| height: 24px; | ||
| width: 24px; | ||
| } | ||
| .arrow { | ||
| height: 20px; | ||
| color: var(--surface-six); | ||
| transform: rotate(0deg); | ||
| } | ||
| .arrow :global(svg) { | ||
| width: 20px; | ||
| height: 20px; | ||
| } |
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.
Indent these
| .donate-card { | ||
| text-decoration: none; | ||
| background-color: var(--surface-nine); | ||
| border-radius: 1.5rem; | ||
| width: 100%; | ||
| cursor: pointer; | ||
| text-align: left; | ||
| border: none; | ||
| overflow: hidden; | ||
| transition: | ||
| border-radius 0.3s var(--bezier-one), | ||
| background-color 0.3s var(--bezier-one); | ||
| display: block; | ||
| font-family: inherit; | ||
| padding: 0; | ||
| } | ||
| .donate-card:hover { | ||
| background-color: var(--tertiary); | ||
| } | ||
| .donate-card:active { | ||
| border-radius: 2.75rem; | ||
| } | ||
| .card-image { | ||
| height: 200px; | ||
| width: 100%; | ||
| object-fit: cover; | ||
| object-position: center; | ||
| display: block; | ||
| } | ||
| .card-label { | ||
| display: block; | ||
| color: var(--text-four); | ||
| font-size: 1.05rem; | ||
| font-weight: 500; | ||
| padding: 1.5rem; | ||
| } |
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.
Indent these
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.
This isnt needed
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.
This isn't needed
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.
Why are these separated now?
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.
They handle different parts of the edit form. Made editing logic easier to manage, but could be merged back if preferred but one single file will be large (600+ lines).
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.
That wasn't the case here:
https://github.com/ReVanced/revanced-website/blob/main/src/routes/announcements/%5Bslug%5D/Content.svelte
It handled both end-user view and admin view
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.
Why are these separated now?
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.
Why are these separated now?
src/routes/donate/+page.svelte
Outdated
| let isDonateLoading = $derived(aboutQuery.isPending && donationLinks.length === 0); | ||
| let isDonateError = $derived(aboutQuery.isError && donationLinks.length === 0); | ||
| let isTeamLoading = $derived(teamQuery.isPending && teamMembers.length === 0); | ||
| let isTeamError = $derived(teamQuery.isError && teamMembers.length === 0); |
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.
This isn't needed, please don't bloat the repository with skeletons, that's not part of the rewrite
src/routes/donate/+page.svelte
Outdated
| {#if isDonateLoading} | ||
| <div class="loading-state"> | ||
| <div class="skeleton-cards"> | ||
| <div class="skeleton-card"></div> | ||
| <div class="skeleton-card"></div> | ||
| <div class="skeleton-card"></div> | ||
| </div> | ||
| </div> |
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.
Skeletons don't belong in the rewrite, this should be an individual feat aiming to follow best practices using proper libraries
src/routes/donate/+page.svelte
Outdated
| .skeleton-cards { | ||
| display: flex; | ||
| gap: 1rem; | ||
| } | ||
| .skeleton-card { | ||
| width: 200px; | ||
| height: 120px; | ||
| background: linear-gradient(90deg, var(--surface-three) 25%, var(--surface-four) 50%, var(--surface-three) 75%); | ||
| background-size: 200% 100%; | ||
| border-radius: 12px; | ||
| animation: shimmer 1.5s infinite; | ||
| } | ||
| .skeleton-team { | ||
| width: 100%; | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fill, minmax(325px, 1fr)); | ||
| gap: 1rem; | ||
| } | ||
| .skeleton-member { | ||
| height: 80px; | ||
| background: linear-gradient(90deg, var(--surface-three) 25%, var(--surface-four) 50%, var(--surface-three) 75%); | ||
| background-size: 200% 100%; | ||
| border-radius: 12px; | ||
| animation: shimmer 1.5s infinite; | ||
| } |
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.
This is too much to maintain, please remove along with skeletons
src/routes/donate/+page.svelte
Outdated
| .hero { | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| gap: 2rem; | ||
| } | ||
| .hero-text h2 { | ||
| margin-bottom: 0.5rem; | ||
| color: var(--text-one); | ||
| } | ||
| .highlight { | ||
| color: var(--primary); | ||
| } | ||
| .hero-text p { | ||
| margin-bottom: 2rem; | ||
| width: 60%; | ||
| color: var(--text-four); | ||
| } |
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.
Use indentation inside braces
.hero {
p {}
}
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.
This file has grown substantially, why?
…ng/error states, and clean up formatting across announcement, donate, and contributor pages.
…nts.svelte.ts to use PersistedState directly from runed
No description provided.