Skip to content

don't forget we are forking from local#343

Merged
paul90 merged 5 commits intomainfrom
fork-from-local
Jun 18, 2025
Merged

don't forget we are forking from local#343
paul90 merged 5 commits intomainfrom
fork-from-local

Conversation

@paul90
Copy link
Member

@paul90 paul90 commented Jun 7, 2025

if we remember we're forking from local the change from local to view in the location bar, and elsewhere, will get fixed up correctly.

@paul90 paul90 requested a review from WardCunningham June 10, 2025 09:44
Copy link
Member

@WardCunningham WardCunningham left a comment

Choose a reason for hiding this comment

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

It looks like there was a plan once. Nice to see it come to completion.

@paul90
Copy link
Member Author

paul90 commented Jun 12, 2025

Further testing shows this is not quite right.

pageHandler.useLocalStorage = () => $('.login').length > 0 is a hang over from openID, and is always going to be false. So, pagePutInfo.site === 'local' is the only thing that governs if we are saving to local, other than the error handling in pushToServer().

I think the original intention could have been to save to local if not logged in and also if the edit as on a page specifically loaded from local.

paul90 added 4 commits June 12, 2025 17:21
use `isOwner` as a hint if the user is the wiki owner. this is set initially by the server.

If the value is incorrectly `true`, it just means there will be a server roundtrip before writing to local storage.
don't push to local if this is a fork action.
@paul90 paul90 requested a review from WardCunningham June 17, 2025 10:38
@paul90 paul90 merged commit 77149d7 into main Jun 18, 2025
3 checks passed
@paul90 paul90 deleted the fork-from-local branch June 18, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants