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.

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

feat(glyphs): virtual list glyph pack placeholder images #700

Merged
merged 7 commits into from
Dec 22, 2021
Merged

feat(glyphs): virtual list glyph pack placeholder images #700

merged 7 commits into from
Dec 22, 2021

Conversation

josephmcg
Copy link
Contributor

@josephmcg josephmcg commented Dec 20, 2021 •
edited
Loading

What this PR does 📖

  • implement virtual list for glyph packs to reduce DOM nodes. more-so a future proof fix when we have hundreds of packs.
  • removed lazy loading for glyphs in favor of virtual list.
  • Add placeholder img while glyphs are loading. I made it myself, maybe Liz could make something better looking. I did this rather than a loader component because that would require mounting and unmounting a component for every image (125 at the moment).
  • refactored filter function to be easier to read
  • changed Recently Used glyphs to send functionality rather than filter
  • removed Most Used pack at the top since it is now very similar to Recently Used.
  • removed broken glyph links until they get fixed

Which issue(s) this PR fixes 🔨
AP99, AP250

Special notes for reviewers 🗒️

Additional comments 🎤

  • Virtual list was somewhat picky about the Props, hence the PackGroup file
  • Right now the Recently Used glyphs are static, I can make them dynamic later.
  • Recently used has too many items on mobile, can also clean that up
  • scroll inside enhancers is a little buggy. emojis and glyphs have a few layout differences. I can fix in a future ticket

@josephmcg josephmcg changed the title feat: virtual list for glyph packs and placeholder images while loading AP-99 feat(glyphs): virtual list for glyph packs and placeholder images while loading AP-99 Dec 20, 2021
@Satellite-im Satellite-im deleted a comment from github-actions bot Dec 20, 2021
@Satellite-im Satellite-im deleted a comment from github-actions bot Dec 20, 2021
@josephmcg josephmcg changed the title feat(glyphs): virtual list for glyph packs and placeholder images while loading AP-99 feat(glyphs): virtual list for glyph packs and placeholder images while loading AP-99 AP-250 Dec 20, 2021
@josephmcg josephmcg added the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Dec 20, 2021
@josephmcg josephmcg removed the request for review from InfamousVague December 20, 2021 07:41
WanderingHogan
WanderingHogan reviewed Dec 20, 2021
View reviewed changes
mock/glyphs.ts
`${Config.ipfs.gateway}QmRBxCyQa2yJFpF4yiV2wimLX2i32oty8yKFJFsFhyJR2n/JohnTreanor_GenshinImpactFood_20.png`,
`${Config.ipfs.gateway}QmRBxCyQa2yJFpF4yiV2wimLX2i32oty8yKFJFsFhyJR2n/JohnTreanor_GenshinImpactFood_21.png`,
// `${Config.ipfs.gateway}QmRBxCyQa2yJFpF4yiV2wimLX2i32oty8yKFJFsFhyJR2n/JohnTreanor_GenshinImpactFood_22.png`,
// `${Config.ipfs.gateway}QmRBxCyQa2yJFpF4yiV2wimLX2i32oty8yKFJFsFhyJR2n/JohnTreanor_GenshinImpactFood_23.png`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Miru will send us these this week

@WanderingHogan
Copy link
Contributor

your pr template is a thing of beauty

@stavares843
Copy link
Member

neat! 🎉

@stavares843 stavares843 added the QA tested One QA Person has tested - Needs QA Lead review still label Dec 20, 2021
@WanderingHogan WanderingHogan added the temporary blocked checking something QA Lead is checking something. label Dec 20, 2021
@github-actions github-actions bot added the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Dec 21, 2021
@WanderingHogan WanderingHogan removed Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA temporary blocked checking something QA Lead is checking something. labels Dec 22, 2021
@josephmcg
feat(glyphs): virtual scroll with styling and filter
eb820b3 
* removed Most used component in favor of improving Recently Used soon
* added virtual scroll to glyphs
@josephmcg
feat(recent area): changed functionality of recently used glyphs
822e67c 
*adjusted to sending, rather than filtering
*removed overflow offscreen by reducing glyph size
*removed filter click events from glyph nav, only search filters
@josephmcg
feat(glyphs): placeholder img src on glyph load
745bf01
@josephmcg
feat(glyph modal): add loading img for glyph modal
9ff7f33
@josephmcg
fix(enhancers): responsive style fixes
1865e90
@josephmcg
remove glyph margin
ccae53b
@josephmcg
fix scroll issue
65a8d81
@WanderingHogan WanderingHogan removed the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Dec 22, 2021
@stavares843 stavares843 changed the title feat(glyphs): virtual list for glyph packs and placeholder images while loading AP-99 AP-250 feat(glyphs): virtual list glyph pack placeholder images Dec 22, 2021
@stavares843 stavares843 merged commit 247ad55 into dev Dec 22, 2021
@stavares843 stavares843 deleted the AP99 branch December 22, 2021 15:07
@github-actions github-actions bot removed the QA tested One QA Person has tested - Needs QA Lead review still label Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WanderingHogan WanderingHogan WanderingHogan left review comments

@stavares843 stavares843 stavares843 approved these changes

@Jekrimo Jekrimo Jekrimo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@josephmcg @WanderingHogan @stavares843 @Jekrimo