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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide feature to have prepared shimmer layout #38

Merged
merged 8 commits into from
Feb 2, 2024

Conversation

avalanchas
Copy link

@avalanchas avalanchas commented Jan 25, 2024

Description

This PR adds very simple functionality to give a VeilLayout (and thus by extension a VeilRecyclerFrameView) a pre-made (prepared by the caller ahead of time) ShimmerFrameLayout to show when veiled. This simply means that the custom "masking" process is skipped and the VeilLayout simply shows the defaultChild view - which, again, is already shimmering

This means that the caller can just as well insert a Shimmer Layout for a carousel item, thereby going the first steps towards fixing #15 馃帀
Personally, I would not close #15 yet, because in a perfect world AndroidVeil would auto-mask a carousel layout as well

A demo activity for a shimmering carousel is included in this PR

Types of changes

What types of changes does your code introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

This PR is fully compatible with previous versions because we simply assume isPrepared = false in all places, named parameters and xml attrs. Thus, the behaviour of existing apps does not change, only when isPrepared is set to true will the code stop adding the masked shimmerContainer

Preparing a pull request for review

  • gradle spotless run
  • gradle apiCheck

add separate activities in demo app, each demonstrates one type of recyclerview
add toolbar navigation and better FAB UI
add "isPrepared" to attrs
Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Hey @avalanchas, this is very impressive and well-refactored across overall architecture!

I do love this PR 鉂わ笍 I really appreciate your huge contribution! It must be very helpful to these library users.

@skydoves skydoves merged commit 3bd2f41 into skydoves:main Feb 2, 2024
1 of 3 checks passed
@avalanchas avalanchas deleted the prepared-shimmer branch February 19, 2024 15:39
@avalanchas
Copy link
Author

Hi @skydoves, thank you for the high praise, coming from you it means a lot! I have one more note though: as I mentioned above:

Personally, I would not close #15 yet, because in a perfect world AndroidVeil would auto-mask a carousel layout as well

In my humble opinion I think we should reopen #15 until carousels can be auto-masked
I hope I can get to this in the next few months

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.

how to LinearLayoutManager is LinearLayoutManager.HORIZONTAL
3 participants