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

Minor corrections in the menu #1631

Merged
merged 8 commits into from
Nov 7, 2023

Conversation

mkz212
Copy link
Contributor

@mkz212 mkz212 commented Nov 5, 2023

  • Adding Logs to the menu.
  • Change the name from Settings to Config Ui Settings
  • Small change of order.

@donavanbecker donavanbecker changed the base branch from latest to beta-4.52.2 November 5, 2023 19:38
@donavanbecker
Copy link
Contributor

Do you have a screenshot of the differences?

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 6, 2023

I will be very happy to do screenshoot. Just tell me where on FTP is the Config Ui plugin directory? /var/lib/homebridge/node_modules/ contains the homebridge directory and other plugins but there is no config ui folder (even after turning on showing hidden files and folders).

It is intended to be something like this:

Terminal
Logs
--------------
Restart server
Close server
--------------
Homebridge Settings
Config Ui Settings
Backup
Users
Logout

or

Terminal
Logs
--------------
Homebridge Settings
Config Ui Settings
Backup
Users
--------------
Restart server
Close server
Logout

What do You think? Which is better? The longer I look at it, the more I have the impression that the second option is better.

@donavanbecker
Copy link
Contributor

Yeah 2nd option

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 6, 2023

@donavanbecker Please tell me what is the path to Config Ui folder?

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 6, 2023

ok I found folder /opt/homebridge/lib/node_modules/homebridge-config-ui-x but there is no layout.component.html file.

Anyway, I just update code to second option.

@donavanbecker donavanbecker enabled auto-merge (squash) November 6, 2023 17:06
@bwp91
Copy link
Contributor

bwp91 commented Nov 6, 2023

My thoughts: do we need the UI Settings in this list? It can be accessed on the plugin screen under settings for the config ui plugin.

That said, if we removed this, we could simply rename the 'Homebridge Settings' to 'Settings'.

I asked chatgpt about ordering!


The ordering of menu options in a drop-down menu can significantly impact user experience. Here's a suggested ordering for the menu options you provided based on their common usage and logical flow:

User Accounts: Placing this option first allows users to manage their accounts, which is often a primary concern for users interacting with a system.

Settings: Users typically access settings to customize their experience, so this option logically follows user accounts.

Backup/Restore: Backup and restore options are usually related to data management. Placing them here allows users to manage their data before performing actions like restart or shutdown.

Terminal: The terminal option is often used by advanced users or developers. Placing it after basic user-related options keeps the menu organized for both regular users and advanced users.

Restart: Restarting the system is a common action. Placing it here ensures that important tasks like managing accounts, customizing settings, and managing data are completed before performing a system restart.

Shutdown: Shutting down the system is a critical action and is typically placed at the bottom of the list to avoid accidental clicks.

Logout: Logging out is usually the last option as it allows users to end their current session. Placing it at the bottom ensures that users consider other options before choosing to log out.

This ordering provides a logical flow from user-specific actions to system-related actions, with critical actions placed at the end to prevent accidental selection. However, the specific ordering can depend on the context and the target audience of your application. Always consider conducting usability testing with your actual users to determine the most intuitive menu order for your particular interface.


However I would suggest swapping the first two so that Settings comes first: ie

  • Settings
  • User Accounts
  • Backup/Restore

  • Terminal
  • Logs

  • Restart Server
  • Shutdown Server
  • Log out

any thoughts?

Having taken another look at my suggestion here, I also feel like shutdown server is too close to log out, could be accidentally clicked and a user may not know how to get it going again, or is away from home and cannot restart.

Maybe we don't even need the shutdown option? Or we merge the two into a 'Restart/Shutdown' option and have the two buttons in a modal that appears, for an extra 'confirmation'.

Maybe I am overthinking now :p

@donavanbecker
Copy link
Contributor

donavanbecker commented Nov 6, 2023

Maybe we don't even need the shutdown option? Or we merge the two into a 'Restart/Shutdown' option and have the two buttons in a modal that appears, for an extra 'confirmation'.

I like the Restart/Shutdown option, extra confirmation is always good! Call it Power.

or we could just have a confirmation modal that pops up for both of them?

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 6, 2023

  • I like the idea of removing Config Ui Settings and naming Homebridge Setting as Settings. But it is not necessary - just changing the name from the current Settings to Config Ui Settings will improve the whole.
  • I would rather leave the Terminal and Logs at the top because the Settings (and certainly Users) are probably less frequently visited than the Terminal and Logs.
  • I like the extra confirmation too.

@bwp91
Copy link
Contributor

bwp91 commented Nov 7, 2023

Screenshot 2023-11-07 at 11 03 16

I think the UI Settings is okay as it is? The config-ui-x package is becoming more and more referenced to as 'Homebrudge UI' so I think UI Settings is okay?

The confirmation modals for restart and shutdown already exist.

I wonder why we need the extra logs item since there is an icon and link to the logs page at a higher level next to the 3 dots which opens the menu. Which I understand my thought here is inconsistent with the restart icon at the higher level, and also a restart menu item.

Perhaps this whole section needs an overhaul

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 7, 2023

  • restart the server is both in the menu (3 dots) and directly on the bar. That's why I added Logs to ensure consistency.

  • about the order and change name, I agree with @donavanbecker (see the statement above) that such an rearrangement and name change is in good solution.

  • when you click restart server in menu (3 dots) there is confirmation modal, but when you click directly in navbar there is no confirmation.

  • About this button to restart (directly in navbar) maybe this icon should be "arrows-rotate" (power button icon suggest something like shut down).

Zrzut ekranu 2023-11-7 o 13 15 32

@bwp91
Copy link
Contributor

bwp91 commented Nov 7, 2023

  • restart the server is both in the menu (3 dots) and directly on the bar. That's why I added Logs to ensure consistency.

👍

  • about the order and change name, I agree with @donavanbecker (see the statement above) that such an rearrangement and name change is in good solution.

👍 Happy with the order, but Im still keen on keeping the text for the ui settings as UI Settings

  • when you click restart server in menu (3 dots) there is confirmation modal, but when you click directly in navbar there is no confirmation.

👍

  • About this button to restart (directly in navbar) maybe this icon should be "arrows-rotate" (power button icon suggest something like shut down).
Zrzut ekranu 2023-11-7 o 13 15 32

Hmm Id rather keep it as it is, otherwise a user might simply it acts as a refresh button (as it looks like a browser refresh button)

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 7, 2023

Change to UI Settings? I think it is even better.

@bwp91
Copy link
Contributor

bwp91 commented Nov 7, 2023

Screenshot 2023-11-07 at 15 56 11

It is already UI Settings

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 7, 2023

So mayby it is translation problem because in pl it is just "Ustawienia" ("Settings"). There is no "UI".

@bwp91
Copy link
Contributor

bwp91 commented Nov 7, 2023

Yeah it's probably just how it's been translated in the pl.json file

auto-merge was automatically disabled November 7, 2023 16:05

Head branch was pushed to by a user without write access

@mkz212
Copy link
Contributor Author

mkz212 commented Nov 7, 2023

Ok now it is: UI + translated settings. UI is hardcoded. settings is translated. So in EN it will be UI Settings, in PL it will be UI Ustawienia.

@donavanbecker donavanbecker merged commit a7e8f54 into homebridge:beta-4.52.2 Nov 7, 2023
13 checks passed
@mkz212 mkz212 deleted the menu-layout branch November 9, 2023 08:16
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.

3 participants