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.

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

fix: prevent re-render on issue fetching and improve PWA #136

Merged
merged 13 commits into from
Oct 29, 2024
4 changes: 2 additions & 2 deletions src/home/fetch-github/fetch-and-display-previews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ function getProposalsOnlyFilter(getProposals: boolean) {
}

// checks the cache's integrity, sorts issues, checks Directory/Proposals toggle, renders them and applies avatars
export async function displayGitHubIssues(sorting?: Sorting, options = { ordering: "normal" }) {
export async function displayGitHubIssues(sorting?: Sorting, options = { ordering: "normal" }, skipAnimation = false) {
await checkCacheIntegrityAndSyncTasks();
const cachedTasks = taskManager.getTasks();
const sortedIssues = sortIssuesController(cachedTasks, sorting, options);
const sortedAndFiltered = sortedIssues.filter(getProposalsOnlyFilter(isProposalOnlyViewer));
renderGitHubIssues(sortedAndFiltered);
renderGitHubIssues(sortedAndFiltered, skipAnimation);
applyAvatarsToIssues();
}
8 changes: 6 additions & 2 deletions src/home/fetch-github/fetch-issues-full.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ export async function postLoadUpdateIssues() {
const fetchedIssues = await fetchIssues();

if (issuesAreDifferent(cachedIssues, fetchedIssues)) {
await saveIssuesToCache(cachedIssues, fetchedIssues);
await saveIssuesToCache(cachedIssues, fetchedIssues); // this handles stale and new issues
await taskManager.syncTasks();
void displayGitHubIssues();
if (cachedIssues) {
void displayGitHubIssues(undefined, undefined, true); // if there were cached issues skip animation
Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to change the function signature so that you don't have to pass in two undefined?

Alternatively, if it makes sense, pass in destructured parameters (an object)

} else {
void displayGitHubIssues(); // if it's first time loading keep animation
}
}
} catch (error) {
console.error("Error updating issues cache", error);
Expand Down
10 changes: 7 additions & 3 deletions src/home/rendering/render-github-issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { closeModal, modal, modalBodyInner, titleAnchor, titleHeader } from "./r
import { setupKeyboardNavigation } from "./setup-keyboard-navigation";
import { isProposalOnlyViewer } from "../fetch-github/fetch-and-display-previews";

export function renderGitHubIssues(tasks: GitHubIssue[]) {
export function renderGitHubIssues(tasks: GitHubIssue[], skipAnimation: boolean) {
const container = taskManager.getContainer();
if (container.classList.contains("ready")) {
container.classList.remove("ready");
Expand All @@ -22,8 +22,12 @@ export function renderGitHubIssues(tasks: GitHubIssue[]) {
if (!existingIssueIds.has(task.id.toString())) {
const issueWrapper = everyNewIssue({ gitHubIssue: task, container });
if (issueWrapper) {
setTimeout(() => issueWrapper.classList.add("active"), delay);
delay += baseDelay;
if (skipAnimation) {
issueWrapper.classList.add("active");
} else {
setTimeout(() => issueWrapper.classList.add("active"), delay);
delay += baseDelay;
}
}
}
}
Expand Down
39 changes: 15 additions & 24 deletions static/progressive-web-app.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const cacheName = "pwacache-v2"; // Increment this when files change
const cacheName = "pwacache-v3"; // Increment this when files change
const urlsToCache = [
"/",
"/index.html",
Expand All @@ -21,7 +21,6 @@ self.addEventListener("install", (event) => {
})
.catch((error) => console.error("[Service Worker] Cache failed:", error))
);
self.skipWaiting(); // activate the new worker immediately
});

// Activate event (deletes old caches when updated)
Expand All @@ -43,29 +42,21 @@ self.addEventListener("activate", (event) => {
self.clients.claim(); // Take control of all pages immediately
});

// Fetch event: Respond from cache or network
// Fetch event (try network first, if offline return from cache)
self.addEventListener("fetch", (event) => {
const url = new URL(event.request.url);

// If the request has query parameters, bypass the cache
if (url.search) {
event.respondWith(fetch(event.request));
return;
}

event.respondWith(
caches.match(event.request).then((cachedResponse) => {
if (cachedResponse) {
return cachedResponse;
}
return fetch(event.request)
.then((networkResponse) => {
return networkResponse;
})
.catch((error) => {
console.error("[Service Worker] Network request failed:", error);
return null;
fetch(event.request)
.then((networkResponse) => {
return networkResponse;
})
.catch(() => {
return caches.match(event.request).then((cachedResponse) => {
if (cachedResponse) {
return cachedResponse;
} else {
return null;
}
});
})
})
);
zugdev marked this conversation as resolved.
Show resolved Hide resolved
});
});
Loading