New patch: Settings Display Within Menus. DONE!

[fatherfork]

Worst case scenario: font has no margin to the left, but 1px to the right - then you've got 2 empty pixels to the right and none to the left! 

Why not force a 1px margin to the left as well?

FF

[AM]

Worst case scenario: font has no margin to the left, but 1px to the right - then you've got 2 empty pixels to the right and none to the left! 

Why not force a 1px margin to the left as well?
FF

Heh, if you had continued reading the second paragraph of that post you'd see that the problem with this is it sort of screws up scrolling:

And working around this by just forcing an additional 1px margin to the left will screw up the scrolling. When the text starts scrolling it wont disappear behind the left edge of the screen or behind the scrollbar, it'll disappear 1 pixel away from it.

Apparently some feel this is ok but I think others (including myself) would be bothered by it...

[fatherfork]

oops, I did read that too. I personally wouldn't have a problem with the forced left margin, but would be satisfied either way. Now If I can only figure out how to compile patches (not a question).

FF

[fml2]

Interesting...I would suggest modeling it after the "Show ID3 Info" screen (it's a list were each item is 2 lines). It might look cleaner and scale better to other targets (particularly those with smaller screens).

That looks nice at the first glance. But there are so many menu items in RB that I'd rather had one line per item. This way I'll find the desired item faster.

Another possibility: make menu display mode an option (with/without current value).

[AM]

Latest update:

• Removed the 1px right margin for now, probably won't add the margins again until I've figured out a better way to do this.
• Added a Boolean Quick Toggle option: when enabled selecting a boolean setting toggles the value instead of entering edit mode.
• Converted all of General Settings. (well almost all, anything still left as ordinary menus was done so intentionally)

TODO now: Lots of testing on different targets...

Another possibility: make menu display mode an option (with/without current value).

Well, that IS the way it's done for now, you have the following options:

• Don't display values in menus, one line per setting (like the current unpatched rockbox)
• Display value only for the selected setting, one line per setting
• Display values for all settings, one line per setting
• Display values for all settings, two lines per setting

[AM]

Converted the rest of the main menu, plus the playlist viewer context menu.
Menu conversion is now complete! yay!

[ryran]

This looks really nice AM. I definitely look forward to checking it out the next time I update my build.

[psycho_maniac]

This looks really nice AM. I definitely look forward to checking it out the next time I update my build.

yes this is truly amazing !!! keep up the good work

[ww2-]

Anyone tested this on the iPod 5G?

[AM]

Anyone tested this on the iPod 5G?

Should work. Works just fine in the simulator.

[Rincewind]

I get a compile error with your patch.

*** No rule to make target 'select.c'

You added 'select.c' to apps/SOURCES, but this file is located in apps/gui/
I changed the line to 'gui/select.c'. This worked, but I get a lot of compiler warnings and some more errors.
Do you get them too or is there a problem with my build?

I only applied your patch and tried to compile a H120 Simulator

[AM]

I get a compile error with your patch.

*** No rule to make target 'select.c'

You added 'select.c' to apps/SOURCES, but this file is located in apps/gui/
I changed the line to 'gui/select.c'. This worked, but I get a lot of compiler warnings and some more errors.
Do you get them too or is there a problem with my build?

I only applied your patch and tried to compile a H120 Simulator

Damn. Actually no, the problem was that in the latest patch I forgot to include select.c and select.h  , these are new files that should be in apps. The select.c and select.h in apps/gui/ on the other hand have been obsolete since the setting selection list was included in CVS and should be deleted. I've now uploaded the (hopefully this time) corrected patch.

[Rincewind]

It is working now.

One small suggestion: How about adding the submenu indicator ( > ) in the "always" and "for selected only" modes, too?

[AM]

It is working now.

One small suggestion: How about adding the submenu indicator ( > ) in the "always" and "for selected only" modes, too?

I've done some thinking about that, but I'm still undecided. Theres already the risk of the screen feeling cluttered with the single line setting as it is, and turning it on for those modes too increases that further imo.

While on the subject, currently the ">" doesn't actually tell you it's a submenu, just that it's not a setting. Do we want that to be handled differently for menu items that actually open submenus vs ones that perform actions? And should there in that case be a different indicator for menu items that perform actions or just no indicator at all (personally I don't like the look of empty lines in a menu)?

If we opt for any of this I'd be happy to implement support for this, but I don't feel like going through all the menus once again to designate which options are actions vs. submenus. It would be dead easy to (anyone could do it), just tiresome and time consuming...

[Rincewind]

differentiating between actions and submenu items would be useful.
My first thought for actions was an asterisk * but I don't know if I really like it.
There are not that many actions, i can only think of Radio, Recording Screen, Graphic EQ, Rockbox Info, View Current Playlist.
But there is Browse cfg files, plugins, themes etc, which is not really a submenu. I don't know how we should handle these.

I just don't like it when there is 'something' for some items and nothing for others on the right side of the screen.

btw, a graphic for > would be cool, but this has to be in different font sizes...