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

Move Notebooks to bottom tabs #146

Merged
merged 1 commit into from
Feb 21, 2021
Merged

Move Notebooks to bottom tabs #146

merged 1 commit into from
Feb 21, 2021

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Feb 1, 2021

This (among other things) would simplify the work on #106.

We now use the metadata of the notebooks: color and description.

notebooks

Tested on web Firefox Linux
Tested on native Android

@tasn
Copy link
Member

tasn commented Feb 1, 2021

How is the bottom sheet better than the side sheet? I'm not objecting per se, I'm just trying to understand what it's here to solve.
File explorer sounds better, though it's a shift in how we do things. I kind of like the flatness of it all at the moment. Though maybe an hierarchy is better. Do we merge different notebooks or not in the view? These are all questions that need answering... I have no idea.

@zecakeh
Copy link
Contributor Author

zecakeh commented Feb 1, 2021

How is the bottom sheet better than the side sheet? I'm not objecting per se, I'm just trying to understand what it's here to solve.

From #106:

The notebook selection is not implemented. We can't use the URL to change the notebook. This makes the navigator navigate to Home, so the screen on the right doesn't stay open. Plus it's a weird behavior to have the URL change when only the list changes.
It is also difficult to send data from the Drawer to the Notes List Screen. We could use a Context but that's seems exaggerated just for this.

The bottom sheet is better because it's part of NoteListScreen so the notebook change is controlled by the screen.

File explorer sounds better, though it's a shift in how we do things. I kind of like the flatness of it all at the moment. Though maybe an hierarchy is better.

I'm not a fan of a hierarchy view either, but we have a lot of other possibilities. I've been looking at the different Google apps on my phone (Calls, Messages, Photos, Drive…) and they all have different modern UIs that could be applicable to our use case: search-bar that shows categories when the search bar is focused (Messages), tabs for the different ways to browse the content (Photos), both (Calls and Drive). We just have to pick one we like.

Do we merge different notebooks or not in the view?

I'm not sure what you mean.

@zecakeh zecakeh changed the title Move notebooks list to a bottom sheet WIP: Change notebooks UI/UX Feb 2, 2021
@zecakeh zecakeh marked this pull request as draft February 2, 2021 16:15
@zecakeh zecakeh marked this pull request as ready for review February 19, 2021 08:52
@zecakeh zecakeh changed the title WIP: Change notebooks UI/UX Move Notebooks to bottom tabs Feb 19, 2021
@zecakeh
Copy link
Contributor Author

zecakeh commented Feb 19, 2021

This is now based on #158 so should be merged after.

Ready for review.

@tasn
Copy link
Member

tasn commented Feb 21, 2021

It's conflicting now for whatever reason... I just merged #158 as requested.
Anyhow, can we make the text always show? Even for non-active tabs?

@zecakeh
Copy link
Contributor Author

zecakeh commented Feb 21, 2021

Ah I thought that it would work since they should have the same commits… I'll resolve the conflicts then.

Yes we can make the text always show, I just thought it made it clearer which tab is active, since it's inexpensive to switch tabs to see the other labels.

@zecakeh
Copy link
Contributor Author

zecakeh commented Feb 21, 2021

Rebased and ready to merge. I made the labels always show.

@tasn tasn merged commit 83dcc33 into etesync:master Feb 21, 2021
@zecakeh zecakeh deleted the notebooks branch February 21, 2021 20:06
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.

2 participants