Skip to content

Graph cache #72

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.

Sign up for GitHub

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

Jump to bottom
Open
fdietze wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Graph cache #72

fdietze wants to merge 2 commits into from

Conversation

@fdietze
Copy link
Member

@fdietze fdietze commented Jun 22, 2019 •
edited
Loading

  • cache empty page because of channels
  • load latest graph when coming online
  • store current graph when closing browser window
  • store current graph when switching focus (all switches need to be triggered by GlobalState.focus())
  • safe access of localstorage -> internal: Option[Storage] ?
  • invalidate cache on user-change (store all pageids in one storage key)

cornerman
cornerman reviewed Jun 22, 2019
View reviewed changes
webApp/src/main/scala/wust/webApp/ClientStorage.scala Outdated
}
}

def getGraph(page: Page): Option[Graph] = page.parentId.flatMap(getGraph)
Copy link
Member

@cornerman cornerman Jun 22, 2019

Choose a reason for hiding this comment

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

We might want to cache empty pages, too. Because it contains all channels and the user node - is called when switching to welcome view.

cornerman
cornerman reviewed Jun 22, 2019
View reviewed changes
webApp/src/main/scala/wust/webApp/ClientStorage.scala Outdated
}

def getGraph(page: Page): Option[Graph] = page.parentId.flatMap(getGraph)
def getGraph(pageId: NodeId): Option[Graph] = {
Copy link
Member

@cornerman cornerman Jun 22, 2019

Choose a reason for hiding this comment

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

We need to check canAccessLS before all this methods, so we do not crash when LocalStorage is disabled in a browser. Maybe make internal an option?

@cornerman
Copy link
Member

cornerman commented Jun 22, 2019

Generally LGTM 💟, I just added two comments.

@fdietze fdietze added the todo label Nov 19, 2019
@fdietze fdietze force-pushed the graph-cache branch 2 times, most recently from fe7489f to 8b83a7a Compare November 30, 2019 23:45
@fdietze
WIP: ClientStorage.scala,GlobalState.scala,UrlConfig.scala,AuthContro…
f9982e3 
…ls.scala,Components.scala,CreateNewPrompt.scala,DoodleView.scala,MembersModal.scala,NewProjectPrompt.scala
@fdietze
WIP: build.sbt,Deps.scala,plugins.sbt,ClientStorage.scala,Base64Codec…
7189e69 
….scala,Notifications.scala,UrlConfig.scala,BinaryConvertors.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cornerman cornerman cornerman left review comments

Assignees

No one assigned

Labels

todo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fdietze @cornerman