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

Style tweaks: move some sub-menus to first-level menu #4384

Merged
merged 4 commits into from Dec 7, 2018

Conversation

Projects
None yet
2 participants
@poire-z
Copy link
Contributor

poire-z commented Dec 7, 2018

It just occured to me that we don't need yhe first-level menu to be short if it contains no tweak itself (only the submenus with toggable tweaks need to be kept shorter to let room below to see the tweak effects).

For now with this PR (just moved Tables and Images out from Miscellaneous>)
image
We have room for 8 submenus containers of tweaks (10 minus the top info item minus the bottom user style tweaks) if we want them all on a single page.

The only submenu that contains itself submenus is Text>:
image

So, we have room for the 3 submenus from Text> (but Text> followed by Text alignment> would look a bit strange - we could keep it inside Text>, no real need to move them all to top.)
I'd like to keep a slot for a (for now removed) Miscellaneous>.

We could then have:

Enable style tweaks (hold for info)
--------
Page>
Paragraphs>
Text>
Text alignment>
Links>
Tables>
Images>
Miscellaneous>
--------
User style tweaks

Any idea/comment about order and titles ?

@poire-z poire-z changed the title Style tweaks: move some sub-menus to first-leel menu Style tweaks: move some sub-menus to first-level menu Dec 7, 2018

Style tweaks: move some sub-menus to first-level menu
As we don't need that first-level menu to be short
if it contains no tweak itself (only the submenus
with toggable tweaks need to be kept shorter to
let room below to see the tweak effects).

@poire-z poire-z force-pushed the poire-z:tweaks_menu branch from 56b358e to f38bbf4 Dec 7, 2018

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 7, 2018

The titles seem decent to me, and agreed that text alignment can simply go inside text.

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 7, 2018

OK, so that would give this:
image

(Made Pages and Paragraphs plural, sounds better to me)

@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 7, 2018

On the one hand I think that perhaps it should go text, paragraphs, pages, on the other hand that would cause more of a disconnect with the links/tables/images.

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 7, 2018

I guess I initially thought/went from outer/larger to inner/smaller elements, so Pages went first (and it contains some stuff about page-breaks which are a bit orthogonal to that container hierarchy).

Or:

Enable style tweaks (hold for info)
--------
Pages>
Text>
Paragraphs>
Tables>
Links>
Images>
Miscellaneous>
--------
User style tweaks
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 7, 2018

I think that feels more logical somehow, but either way it doesn't matter that much. :-)

@poire-z

This comment has been minimized.

Copy link
Contributor

poire-z commented Dec 7, 2018

Feels more logical to me too, so let's do that.
image

@poire-z poire-z merged commit 74c3c90 into koreader:master Dec 7, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:tweaks_menu branch Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment