Skip to content

New configuration UI, small fixes#83

Merged
infeeeee merged 16 commits intorichibrics:mainfrom
infeeeee:UnifiedRun
Nov 28, 2023
Merged

New configuration UI, small fixes#83
infeeeee merged 16 commits intorichibrics:mainfrom
infeeeee:UnifiedRun

Conversation

@infeeeee
Copy link
Copy Markdown
Collaborator

@infeeeee infeeeee commented Nov 27, 2023

Configurator rewritten from the ground up.

I used this Inquirer implementation: InquirerPy Docs It had more features than the other one mentioned in #32

In MenuPreset I removed the modify_value_callback argument. The current usage of this tag was replaced by question_type, and InquirerPy has very powerful filters to sanitize the inputs: Filter docs

In MenuPreset, unused configuration options are not saved, simply omitted from the json. As per #73 they are not needed, we read them from defaults on startup.

TODO, Ideas: (maybe in another PR)


Unrelated to this, standardized some parts of the code:

  • One RunCommand() basically a wrapper around subprocess.run(). There were a lot of very similar implementations around the codebase.
  • OsD constants are much shorter now, easier to remember them. From OperatingSystemDetection.OS_FIXED_VALUE_MACOS to OsD.MACOS

Other files:

  • ClassManager.py, ConfiguratorIO.py: linting
  • Power.py: No separate Callback needed, they can decide it from the topic.

I tested this only on my main computer. Please check if it's working as expected on a Mac.

@richibrics
Copy link
Copy Markdown
Owner

richibrics commented Nov 27, 2023

It's incredible 😍
I'll try on macOS very soon

@richibrics
Copy link
Copy Markdown
Owner

Tested on macOS, I think it's working correctly.

The manage entities page is shows like in the picture, so if you don't scroll down the entire page you don't see the add entity button. What if we move it to the top ? And I'd add a "Active entities" label above the list of entities to express that those are the active entities. What do you think ?
image

@richibrics
Copy link
Copy Markdown
Owner

macOS again: I don't think if it is a bug of the library, but if I am in a page and I press Escape then nothing happens. Then when I press a following key on the keyboard, it comes back to the previous page

@richibrics
Copy link
Copy Markdown
Owner

Update: It comes back with escape If you wait 3 seconds after pressing it. Instead if you press instantly another key after escape there is no delay

@infeeeee
Copy link
Copy Markdown
Collaborator Author

Escape: I also found that it's slow, for me it's not that much, it's a bit annoying, should I remove it?

The manage entities page : That's intended, I mean that's the default settings of max_height, it uses maximum 70% of the screen: https://inquirerpy.readthedocs.io/en/latest/pages/height.html#max-height
I can take a look how it looks like at 100%

@richibrics
Copy link
Copy Markdown
Owner

Entities: okay but what about if we move the Add button to the top ? It would never disappear in this way.
However setting to 100% the max-height (if doesn't look bad) could be an option too, in fact with the white space on the bottom it could seem like there's nothing else under the shown entities.

@richibrics richibrics added enhancement New feature or request WIP labels Nov 27, 2023
@infeeeee infeeeee requested a review from richibrics November 27, 2023 18:30
@infeeeee infeeeee mentioned this pull request Nov 27, 2023
@richibrics
Copy link
Copy Markdown
Owner

Very good work, I read your code and seems good. You really did a great job ! 🚀

@infeeeee
Copy link
Copy Markdown
Collaborator Author

Do not pull yet, I have to update the new Disk entity first

@richibrics
Copy link
Copy Markdown
Owner

Yes sure

@infeeeee
Copy link
Copy Markdown
Collaborator Author

I think it's ready, questions, I'm not sure about:

  • The < Go back button can also move to the top, next to the +Add new button, What do you think is it better if they are on the two sides of the list? I would move it up in every menu.
  • Should I remove the Escape button function in the menu? While using it actually I got used to the small delay.

@richibrics
Copy link
Copy Markdown
Owner

I think for consistency I'd put the Back below the Add. Do you like it ?
I'd also leave the Escape button because however it's a great feature and very useful

@infeeeee
Copy link
Copy Markdown
Collaborator Author

Ok, I will move id up. I had an early version where it was the first in every menu

@infeeeee infeeeee merged commit b48fd2c into richibrics:main Nov 28, 2023
@infeeeee infeeeee deleted the UnifiedRun branch November 28, 2023 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request WIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants