UX: use only one option for settings window

[Fabxx]

Fixes: #1059

showcase.mp4

[antangelo]

I think it's still handy to have the options immediately available. Perhaps a submenu or something could clarify behavior while maintaining the quick access options?

[abaire]

I think it's still handy to have the options immediately available. Perhaps a submenu or something could clarify behavior while maintaining the quick access options?

@antangelo can you expand on your suggestion? Maybe I'm misunderstanding, but given that launching into the general page provides the full list of subpages on the left it seems to me that it's roughly equivalent to a submenu (except in the case where you actually want general settings, in which case it saves a step).

[antangelo]

I whipped this up quickly to demonstrate, shouldn't be hard to isolate it:

It's not a huge deal to click into the settings menu and then use the sidebar to navigate, but I think doing it this way is slightly faster to navigate (saves a click).

I think at minimum, if opening the settings menu is to be done with a single button, it should open the previously opened tab (as opposed to opening the General tab, which is the behavior in this PR from the looks of things).

[Fabxx]

the changes i made make sure that we keep track of the last visited window when exiting and entering back into "Settings", tho if we access Help > About instead of Settings > About, and then we access to Settings, the last value is saved to that window. Let me know if this is not a big issue with the UI design. Also i removed the redundant class functions since we can save now the value of the visited window more generically in one function for "Settings"

[antangelo]

This branch needs to be synced with upstream to resolve conflicts

[Fabxx]

Code has been rebased on the lastes xemu version

[mborgerson]

Thanks for the patch. Since it has been confusing for some users, we can make Settings one single option in the menu. Can you simplify the patch and remove unrelated changes

[Fabxx]

Thanks for the patch. Since it has been confusing for some users, we can make Settings one single option in the menu. Can you simplify the patch and remove unrelated changes

I aborted the modifications that did not regard the actual drop down menu (got confused with the popup menu naming convention). Now we only have the simplified drop-down menu.

[antangelo]

Do you think we should persist the last opened menu tab after xemu restart? This information is already stored in the last_viewed_menu_index config item.

[Fabxx]

Do you think we should persist the last opened menu tab after xemu restart? This information is already stored in the last_viewed_menu_index config item.

i could reset the index to 0 after the first setup, so if a reboot happens the user will be prompted to the first tab of the settings section.

UPDATE: it works if i set to 0 the current_view_index inside the showSystem function, so on next boot you will be prompted to the first window. So tell me if you agree with this approach @antangelo

[antangelo]

I don't think that's quite the same idea that I had. If you were on the snapshots tab, for example, and you close xemu, I think that when you re-open xemu and open settings from the menubar, you should be shown the snapshots tab again.
Do you think this behavior is better than the current behavior (which resets to the general tab)?

[Fabxx]

I don't think that's quite the same idea that I had. If you were on the snapshots tab, for example, and you close xemu, I think that when you re-open xemu and open settings from the menubar, you should be shown the snapshots tab again. Do you think this behavior is better than the current behavior (which resets to the general tab)?

I don't think that's quite the same idea that I had. If you were on the snapshots tab, for example, and you close xemu, I think that when you re-open xemu and open settings from the menubar, you should be shown the snapshots tab again. Do you think this behavior is better than the current behavior (which resets to the general tab)?

The current behavior is this: if the user doesn't move from the settings tab and just restarts it is prompted to the General tab. But if the user moves, on next boot it will be prompted to the last selected window, because the m_current_view_index = 0 gets overrided by the other functions when you change tabs. So i do continue to keep track of the last visited window, but only if the user didn't move from settings on first boot. on next boots this thing won't happen and will just keep track of the last visited window.

This basically means:

-On first boot, press settings from welcome.cc, and reset the index to 0 for next boot, which calls ShowSystem() only once.

-On next boots or if i move in tabs during first boot: call ShowSettings() which doesn't reset the index to 0 and keeps track of the visited window.

The index = 0 thing is not in the code yet

[antangelo]

It keeps track of the visited window within a session, but in my testing if I close xemu and re-open it, the general tab is always shown (regardless of what I had open before).

[Fabxx]

It keeps track of the visited window within a session, but in my testing if I close xemu and re-open it, the general tab is always shown (regardless of what I had open before).

are you writing the index assignation into ShowSettings or ShowSystem?

[antangelo]

I have not modified the code, I just checked out the branch as-is

[Fabxx]

I have not modified the code, I just checked out the branch as-is

oh, well as i said the modification is not here yet. I have to write it now

[antangelo]

I have no idea what that change is supposed to do, or what issue you're trying to address. Whatever you're trying to fix seems to be completely independent from the suggestion I made. m_current_view_index does not retain or restore any value between xemu sessions.

[Fabxx]

I have no idea what that change is supposed to do, or what issue you're trying to address. Whatever you're trying to fix seems to be completely independent from the suggestion I made. m_current_view_index does not retain or restore any value between xemu sessions.

i'm not understanding what's the problem that you're experiencing. I thought the trouble was that after the welcome setup, the user was always redirected to the general tab when opening settings again, so i made it that if the user moves in the tabs after the first setup on next boot it would find himself on the last viewed tab, if he doesn't move he's redirected to general tab. But then again if this is not the issue show me what would you like to do, visually or in code

[antangelo]

1. Open xemu, navigate to Machine -> Settings
2. Select a tab that is not General (e.g. Audio)
3. Close the settings window and then close xemu.
4. Open xemu again, and navigate to Machine -> Settings
5. Observe that the General tab is open, and not the previously selected tab (in the example above, Audio)

I haven't noticed any issues with the welcome flow both with and without your latest change, the system button should always show the system page in that case.

[Fabxx]

understood, will try to fix it

[Fabxx]

OK so the idea i had is to write the last viewed window index into xemu.toml, and then read it when the user boots xemu and initialize the index again to the last visited window. I need some help in understanding how to write to the toml file

[antangelo]

The last viewed tab is already stored in the config for you as last_viewed_menu_index

[Fabxx]

The last viewed tab is already stored in the config for you as last_viewed_menu_index

yeah i found it, is it ok if the function becomes like this? it does keep track of the last visited window on next boots, but i don't know if it's ok enough to assign the value each time showsettings is called. It would make sense if current_index had no value at boot but it is already initialized to 0.

so if the index was set to NULL i could do:

if (m_current_view_index == NULL) {
      m_current_view_index = g_config.general.last_viewed_menu_index;
}
   SetNextViewIndexWithFocus(m_current_view_index);

Current code:

void MainMenuScene::ShowSettings()
{
    m_current_view_index = g_config.general.last_viewed_menu_index;
    SetNextViewIndexWithFocus(m_current_view_index);
}

[antangelo]

You don't need to assign m_current_view_index, it will be updated later. You should be able to just use the config entry as the argument to SetNextViewIndexWithFocus.

[Fabxx]

You don't need to assign m_current_view_index, it will be updated later. You should be able to just use the config entry as the argument to SetNextViewIndexWithFocus.

done it, thanks for the suggestion, let me know if there is anything else to do

[antangelo]

Is this PR now ready for review or is it still in development?