Conversation
There was a problem hiding this comment.
Pull request overview
Removes the ASCII-art banner from the TUI and updates menu rendering to present a simpler centered title and centered menu options.
Changes:
- Removed
banner.hppand replacedrender_banner()usage with a centeredCERTAMENtitle. - Centered main menu entries via a custom
MenuOptionentry transform. - Adjusted quiz picker menu entry strings (removed leading padding) and centered its menu render.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/session.cpp | Drops banner rendering in prompts/picker and tweaks quiz picker entry/menu layout. |
| src/screens/menu.cpp | Removes banner include/usage and centers menu entry labels using a custom MenuOption. |
| src/banner.hpp | Deletes the ASCII-art banner helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return vbox({ | ||
| text(""), | ||
| render_banner(), | ||
| text(" CERTAMEN ") | bold | center, | ||
| text(""), |
There was a problem hiding this comment.
The title element text(" CERTAMEN ") | bold | center is now duplicated in multiple renderers within this file. Consider extracting a small shared helper (e.g., render_title() / constant label element) to avoid repeating styling and to keep future visual tweaks consistent.
| auto option = MenuOption::Vertical(); | ||
| option.entries_option.transform = [](EntryState es) { | ||
| auto label = text(es.label) | center; | ||
| if (es.focused) | ||
| label = label | bold; | ||
| if (es.active) | ||
| label = label | inverted; | ||
| return label; |
There was a problem hiding this comment.
entries_option.transform takes EntryState by value, which forces a copy for each entry render. Switching the lambda parameter to const EntryState& avoids that extra copy and matches typical FTXUI usage.
| Elements content; | ||
| content.push_back(text("")); | ||
| content.push_back(render_banner()); | ||
| content.push_back(text(" CERTAMEN ") | bold | center); | ||
| content.push_back(text("")); | ||
| content.push_back(separator() | color(Color::GrayDark)); |
There was a problem hiding this comment.
The app title element text(" CERTAMEN ") | bold | center is now hard-coded here (and also in other screens). Consider extracting a shared helper/constant for the title to keep UI styling consistent and avoid having to edit multiple call sites for future branding/layout changes.
| entries->push_back(" " + std::filesystem::path(f).stem().string()); | ||
| entries->push_back(std::filesystem::path(f).stem().string()); | ||
|
|
||
| auto menu = Menu(entries.get(), &selected); |
There was a problem hiding this comment.
screens/menu.cpp now centers menu entry labels via a custom MenuOption, but this quiz picker still uses the default Menu(...) rendering (no MenuOption) and only wraps the whole menu with | center. If you want consistent centered options across menus, consider reusing the same MenuOption approach here (or otherwise centering each entry) so the quiz picker and main menu behave the same.
| auto menu = Menu(entries.get(), &selected); | |
| MenuOption menu_option; | |
| menu_option.transform = [](const EntryState& state) { | |
| Element label = text(state.label) | center; | |
| if (state.active) | |
| label |= inverted; | |
| return label; | |
| }; | |
| auto menu = Menu(entries.get(), &selected, &menu_option); |
Remove text banner and center options in menu