Implement the left sidebar #864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sajald77
approved these changes
May 11, 2023
src/hooks/useOnScreen.ts
Outdated
Comment on lines
1
to
28
| import { RefObject, useEffect, useMemo, useState } from 'react' | ||
|
|
||
| export function useOnScreen(ref: RefObject<HTMLElement>) { | ||
| const [isIntersecting, setIntersecting] = useState(false) | ||
|
|
||
| const observer = useMemo( | ||
| () => | ||
| new IntersectionObserver( | ||
| ([entry]) => entry && setIntersecting(entry.isIntersecting), | ||
| { | ||
| threshold: 0.1, | ||
| }, | ||
| ), | ||
| [], | ||
| ) | ||
|
|
||
| useEffect(() => { | ||
| if (!ref.current) { | ||
| return | ||
| } | ||
|
|
||
| observer.observe(ref.current) | ||
| return () => observer.disconnect() | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []) | ||
|
|
||
| return isIntersecting | ||
| } |
There was a problem hiding this comment.
I see you end up not using the useOnScreen hook. We can probably remove it if so.
Also what was the intension here, this seems a little complex method to know if the item is on screen.
It might be easier to check with scrollHeight of the object.
There was a problem hiding this comment.
One more thing, does this include the changes mick mentioned or will that be a separate PR ?
There was a problem hiding this comment.
No it doesn't include that yet
jfrader
merged commit 68a3e8f
into
fran/gey-1721-allow-funder-to-login-from-funding-flow
jfrader
added a commit
that referenced
this pull request
May 12, 2023
* feat: add profile to comment box * fix: missing deps * chore: improve setTarget tn * feat: Implement the left sidebar (#864) * refactor: make ProjectProvider avoid multiple network-only requests not needed * feat: add project navigation in left panel * chore: remove unused hook
jfrader
added a commit
that referenced
this pull request
May 15, 2023
* feat: add profile to comment box * fix: missing deps * chore: improve setTarget tn * feat: Implement the left sidebar (#864) * refactor: make ProjectProvider avoid multiple network-only requests not needed * feat: add project navigation in left panel * chore: remove unused hook
jfrader
added a commit
that referenced
this pull request
May 15, 2023
* feat: add profile to comment box * fix: missing deps * chore: improve setTarget tn * feat: Implement the left sidebar (#864) * refactor: make ProjectProvider avoid multiple network-only requests not needed * feat: add project navigation in left panel * chore: remove unused hook
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves GEY-2204
Refactors the ProjectContainer and ProjectProvider to avoid network-only calls and make navigation between entries and back to project without reloading everything, and also avoiding passing props all the way down at least for the useProjectState. We can improve it even further by doing something similar with the funding flow, avoiding to pass all the setters and getters to every child
Removed several pieces of repeated code that happened when fetching a project, and the fetch itself is now in the ProjectProvider.
Missing the "active" state of the navigation as I still looking for a best way to detect which one should be active. Intersection Observer has performance problems, left commented out for now.