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

Let user decide if items are added to top or bottom of list #32

Open
kaoneko opened this issue Dec 6, 2020 · 2 comments
Open

Let user decide if items are added to top or bottom of list #32

kaoneko opened this issue Dec 6, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@kaoneko
Copy link

kaoneko commented Dec 6, 2020

Wow, this is a pretty decent app! Very compatible with my workflow and looking great!

There's just one thing I notice in a lot of list apps that I can't wrap my head around: why are new items added to the top of the list instead of the bottom? When you guys write a list on a piece of paper or in a text editor do you write from bottom to top?! I'm seriously curious, I guess this is one of those cases where there's two kinds of people...

Anyhow, I would welcome a setting to add new items to the bottom of the list instead of top. Also, if I can easily change this in the source code I'd be glad to build my own customized version of the app. I guess the relevant function is addItem() in OneListFragment.kt? Does this line mean it's added to the top of the list? (Is the 0 the position in the list or something else?)

selectedList.items.add(0, item)

If anyone can tell me how to change it to insert new items at the bottom of the list I'll be happy to hear it!

Edit: found the relevant documentation, changed the line to

selectedList.items.add(item)

and new items are now added to the bottom of the list. Only now they're being covered by the keyboard as long as I'm adding items, because the visible portion of the list still scrolls to the top XD

Edit 2: ...which was easily remedied by changing

itemsRecyclerView.smoothScrollToPosition(0)

to

itemsRecyclerView.smoothScrollToPosition(position)

I was wondering why that 0 was hardcoded when I first saw it 🤔

@lolo-io
Copy link
Owner

lolo-io commented Dec 7, 2020

Hi, thanks four your feedback ! I appreciate it 🙂

There's just one thing I notice in a lot of list apps that I can't wrap my head around: why are new items added to the top of the list instead of the bottom? When you guys write a list on a piece of paper or in a text editor do you write from bottom to top?! I'm seriously curious, I guess this is one of those cases where there's two kinds of people...

To be honest I chose to add the items to the top because it was easier to handle in the code 😄.
As I chose to move the striked items to the end of the list, it was a little bit more work to manage item addition before them, and it was better looking to add them to the top because the keyboard isn't hiding the added item.

I was thinking of adding a setting option to choose whether to add new items to the top or the bottom in the next release. Right now I don't have much time to work on the app.

If you feel like you can do it, then you can open a PR and i'll be glad to review it.
But keep in mind this has to get along with the striked items moving to the bottom of the list. you never want a not striked item below a striked one.

You found the two correct lines, changing them to
selectedList.items.add(item) and itemsRecyclerView.smoothScrollToPosition(position) will let you add items to the end.
But if you do it in a PR you'll have to add a setting to let the user choose the behavior.

If you don't feel like doing a PR I'm not forcing you 😄. I think I'll get back adding features to this app in the early 2021 ;)

I was also thinking of adding a setting to chose whether the striked items move to the bottom or not in the future release.

I was wondering why that 0 was hardcoded when I first saw it

The 0 is there to scroll to the top when you add a new item (which is added to the top), usefull when you have a long list and you are scrolled at the bottom.
The forced scroll is only for UX intentions, the user has to see the item he just added appear on the screen.

@lolo-io lolo-io added the enhancement New feature or request label Dec 7, 2020
@kaoneko
Copy link
Author

kaoneko commented Dec 7, 2020

Ah, I hadn't noticed yet that with my quick hack new items get added after striked items!

As I chose to move the striked items to the end of the list, it was a little bit more work to manage item addition before them

O, you actually move the checked off items to the end of the list, like, the List object that holds all the Item objects? Or am I misunderstanding? I imagine the Item object knows about itself wether it's checked off or not (like it has a boolean isCheckedOff or something) and you could first print all the unchecked items to the screen followed by all the checked items..?

Just thinking out loud here, don't mind my probably foolish ranting. I should write my own list app someday, it'll be an interesting excercise! Or study your code... I don't really know Android development or Kotlin though, so I don't think I'll be generating that PR anytime soon 😄

The 0 is there to scroll to the top when you add a new item (..)

I get this, but since two lines above that you do

val position = selectedList.items.indexOf(item)

I thought it interesting you don't use this variable there. Or maybe this whole line is actually redundant when you're adding the item to index 0. Or I'm just misunderstanding since I didn't study the code very well haha.

Have a good day!

lrq3000 added a commit to lrq3000/OneList that referenced this issue Jan 13, 2023
lrq3000 added a commit to lrq3000/OneList that referenced this issue Jan 13, 2023
lrq3000 added a commit to lrq3000/OneList that referenced this issue Jan 13, 2023
lrq3000 added a commit to lrq3000/OneList that referenced this issue Jan 13, 2023
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

Successfully merging a pull request may close this issue.

2 participants