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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FR]: In the file ScrollbarExt.kt there is room for reducing the logic and code repeating in order to improve the reusability and maintainability #1549

Closed
2 tasks done
rodrigodiasferreira opened this issue Jul 14, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@rodrigodiasferreira
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the problem

In the file ScrollbarExt.kt file there is room for reducing the logic and code repeating in order to improve the reusability and maintainability.
Moreover, this improvement will make it better for developers who use the nia sample to study, and learn ways to use Kotlin to increase resusability.
The scrollbarState extension function for the LazyListState, LazyGridState, LazyStaggeredGridState have a lot of common logic that could be unified.

Describe the solution

My suggestion is to unify into a more generic extension function:
<LazyState : ScrollableState, LazyStateItem> LazyState.genericScrollbarState

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@rodrigodiasferreira rodrigodiasferreira added the enhancement New feature or request label Jul 14, 2024
@rodrigodiasferreira
Copy link
Author

rodrigodiasferreira commented Jul 14, 2024

Hi there,
I would like to contribute to Now In Android sample.
I've already developed such improvement, but I'm not able to upload it.

I've already signed the Google Individual CLA.
I can upload new PRs to my repositories, but for Now in Android it says to check if I have the correct access rights.

I am trying to upload the branch where I developed the improvement: scrollbarext-improvement
git push --set-upstream origin scrollbarext-improvement

This is the error I got:

ERROR: Permission to android/nowinandroid.git denied to rodrigodiasferreira.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.`

Here it is my remote, which looks to be correct:

git remote -v
origin	[email protected]:android/nowinandroid.git (fetch)
origin	[email protected]:android/nowinandroid.git (push)

Anyone can help me to check if I am doing something wrong?

Thanks.

@rodrigodiasferreira rodrigodiasferreira changed the title [FR]: In the file ScrollbarExt.kt file there is room for reducing the logic and code repeating in order to improve the reusability and maintainability [FR]: In the file ScrollbarExt.kt there is room for reducing the logic and code repeating in order to improve the reusability and maintainability Jul 14, 2024
@Jaehwa-Noh
Copy link
Contributor

Follow these steps.

  1. Fork this repository into your repository.
  2. Open 'your forked repository' on the Android Studio.
  3. Edit your code, and push to your forked repository.
  4. Make PR at this origin repository.

@rodrigodiasferreira
Copy link
Author

Hi @Jaehwa-Noh, thanks a lot for the tips.
I've just upload the PR: #1550, I mentioned this issue on the PR, but maybe it should be linked through the development right side bar on this issue, but I don't have permission to do that.
I also do not have permission to self assign this issue to me.

It's my first contribution, so if there is any adjustment, or anything that I should improve, please let me know, I will do my best to conform to your standards.
Thanks!

@rodrigodiasferreira
Copy link
Author

I am closing this issue, as commented by @tunjid on PR: #1550, the split of the functions and logic duplication was done on purpose, in order to improve readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants