Render an empty div for game cards until they inter the viewport #2460
Conversation
This file contains hidden or 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
arielj
requested review from
a team,
flavioislima,
CommandMC,
Nocccer,
imLinguin and
redromnon
and removed request for
a team
arielj
commented
Feb 17, 2023
| } | ||
|
|
||
| .gameCard.gamepad { | ||
| aspect-ratio: 3/4; |
There was a problem hiding this comment.
these are the current ratios, but I'm hardcoding them so the empty divs use the correct space
arielj
commented
Feb 17, 2023
| @@ -63,6 +63,24 @@ const GameCard = ({ | |||
| isRecent = false, | |||
| gameInfo: gameInfoFromProps | |||
| }: Card) => { | |||
| const [visible, setVisible] = useState(false) | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
start as invisible, make visible when the visible-cards event is fired and this card is in the list of visible cards
arielj
commented
Feb 17, 2023
| @@ -344,7 +372,7 @@ const GameCard = ({ | |||
| /> | |||
| )} | |||
| <ContextMenu items={items}> | |||
| <div className={wrapperClasses}> | |||
| <div className={wrapperClasses} data-app-name={appName}> | |||
There was a problem hiding this comment.
using this data attribute for the intersection observer
arielj
added
pr:wip
arielj
commented
Feb 17, 2023
| const { t } = useTranslation() | ||
| const [gameCards, setGameCards] = useState<JSX.Element[]>([]) |
There was a problem hiding this comment.
this useState and the useEffect changing it was causing extra re-renders
I moved this back to the return jsx instead of using a useState+useEffect
arielj
added
pr:ready-for-review
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.
This is an alternative to #2447
Instead of pagination, it renders all the library elements but for the ones that are outside of the viewport it renders an empty div until the element has to be displayed. Then it uses the IntersectionObserver to actually render the real content.
I'll add some comments in the PR.
There are still improvements that can be made (because of how react works, all hooks has to be defined before rendering the empty div, so there's extra unnecessary work there, also cards are re-rendered multiple times) but I'll leave those things for another PR.
You can see the empty divs using devtools and how they are updated when scrolling:
Use the following Checklist if you have changed something on the Backend or Frontend: