Add simple menu for better UX #660

svartkanin · 2021-10-12T08:22:12Z

This PR adds the simple menu (https://github.com/IngoMeyer441/simple-term-menu) as a new selection menu to enhance the UX of the guided user flow.

It has been suggested in issue #655 and this PR changes most selection menus to use the new layout.

The only part where it still uses the old one is during manual partitioning. The reason for that is that I think there are some bugs in the partitioning code and it kept exploding when I was going through the flow in master. Therefore, I decided to leave that one as is and fix the bugs first and only after switch it over to the new layout as well.

Edit: This will fix #173 (I can't link earlier issues to newer PR's because the issues was created before the move from Torxed/archinstall to archlinux/archinstall)

Torxed · 2021-10-12T10:55:21Z

I haven't tested this yet. And I'm generally speaking against external dependencies.
Which is why we've spent some time integrating code that produces menus etc.
Seeing how complex that code is growing, and how shaky especially my handling of input is from time to time - this might enhance the user experience quite significantly.
Perhaps I revisit the refusal to use external libraries and evaluate a plan to keep or remove it in the future - saving us some time on menu management etc.

Again, I have to see how this looks and what it actually does. Just throwing my thought out there that I'm not against it (we just need to keep in mind that some packages are not on the ISO and we don't want to grow the ISO more than we have too even if we're just talking about a few Kb here and there).

svartkanin · 2021-10-12T11:00:19Z

Thanks for the feedback.
All the code in the simple-menu dependency is actually all in a single file. If you want to avoid dependencies then there's the simple option to copy that in manually as well.
Updates have to be done manually as well then tho.

mhussaincov93 · 2021-10-14T14:02:07Z

hi there, blind user here, what inpact will this simple menu have on the accessibility of archinstall? is this simple menu number based? e.g 1 english, 2 spanish, etc? best way of testing the accessibility of these changes is to boot from the arch iso when the boot prompt comes up down arrow once and press enter it will launch the screen reader. I hope that accessibility is not broken with this. Majid

…

On 10/12/21 12:00, Daniel wrote: Thanks for the feedback. All the code in the simple-menu dependency is actually all in a single file. If you want to avoid dependencies then there's the simple option to copy that in manually as well. Updates have to be done manually as well then tho. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#660 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASDFWN6CVZSANME2P4FC26DUGQIM7ANCNFSM5FZ6XYZQ>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

Torxed · 2021-10-14T17:13:56Z

hi there, blind user here, what inpact will this simple menu have on the accessibility of archinstall?

Hi, I'm glad you took this up.
I will make sure to test this! And if it breaks anything I'll report it :)

svartkanin · 2021-10-22T21:55:05Z

Hey @Torxed ,
Did you have a chance to look at this already?

archinstall/lib/user_interaction.py

archinstall/lib/menu.py

Torxed · 2021-10-26T14:08:57Z

Hey @Torxed , Did you have a chance to look at this already?

I've tried to run it a couple of times, due to my debugging setup it took a bit longer than expected.
There are some imports that needs to be cleaned up (i think, please notify if i'm wrong so I can improve).

Once those changes are done, I'll test again and I'll test availability with screen reader as well : )

svartkanin · 2021-10-27T11:04:02Z

@Torxed I updated the PR, I think I addressed all comments. Let me know if you have any other concerns or questions
Cheers

Torxed · 2021-10-27T11:04:21Z

Looks better, while I run a few more tests. Can you updated requirements.txt so that we don't loose the dependency?

svartkanin · 2021-10-27T11:09:52Z

You mean create a requirements.txt ? I did add it to the pyproject.toml but I guess it depends how one installs the dependencies. So I thought installing it with flit install --deps all does the trick there, but honestly I haven't used flit before...

Torxed · 2021-10-27T11:18:23Z

You mean create a requirements.txt ? I did add it to the pyproject.toml but I guess it depends how one installs the dependencies. So I thought installing it with flit install --deps all does the trick there, but honestly I haven't used flit before...

Ah yea my bad. Forgot we moved from requirements.txt to flit installs :) Ignore my previous comment!

svartkanin · 2021-11-01T11:13:24Z

Hey @Torxed ,
did you manage to test this again?
Cheers

Torxed · 2021-11-01T11:25:47Z

Hey @Torxed , did you manage to test this again? Cheers

Not yet, I'll test this one and my other 3-4 PR's I submitted today. Most likely on Wednesday when I'm back at a computer with proper internet. I'm limited to 56k modem speeds for a few days.

svartkanin · 2021-11-01T11:35:37Z

Awesome thanks!

Torxed · 2021-11-05T11:59:47Z

This PR adds a bit of flare to the menu by adding colors and a better interactive selector.
I'm not sure if it's the old if/else we have, but skipping a hard drive will still prompt for wiping or configure individual partitions.

The Text to Speech still works. But it will need some modification to the text output. For instance it's a bit unclear that you can/should use the arrow keys to navigate the menu items, and that you start on the first menu item.
I'm not a frequent TTS user, but this might be an improvement.

One drawback is that it always clears the previous output (something some of our menu screens do as well) and is not desirable I think. It's not the end of the world, but it does mean that some overview of the process gets removed. And with the removal of tty history the config at the end can not be viewed fully either, which maybe makes the clearing of the screen a bit more of a problem today.

I think I'm still in favor of this PR.
But perhaps we hard-link in the single-file menu library to ensure we don't get breakage further down the road?

svartkanin · 2021-11-06T03:27:06Z

Thanks for the review.
I'm happy to include the menu file directly and remove the dependency.
Is there anything else specifically you would like to get changed or updated in this PR?

I'm planning on a next PR to address the remaining old menu selection in the manual partitioning process.

Also I can play around with displaying already chosen options while going through the menu which could serve as a history in a way. But it would be something in a subsequent PR if there's interest to merge this one and worth the effort that way :)

examples/guided.py

profiles/desktop.py

profiles/i3.py

profiles/server.py

Torxed · 2021-11-07T10:54:14Z

Thanks for the review. I'm happy to include the menu file directly and remove the dependency. Is there anything else specifically you would like to get changed or updated in this PR?

I'm planning on a next PR to address the remaining old menu selection in the manual partitioning process.

Also I can play around with displaying already chosen options while going through the menu which could serve as a history in a way. But it would be something in a subsequent PR if there's interest to merge this one and worth the effort that way :)

I think that would be good for now, unless someone else has objections (they might after your finished, it happens) I don't see why this isn't a better step towards handling menus. Instead of trying to save the previous menu options, perhaps we can have a final menu at the end outputting the result config.json data. And tie entries to calling the function responsible for that entry. It would mean keeping and updating a map of what entries comes from which function. But that would allow people to "go back" and re-edit a entry before installing.

Obviously, that would be for the next version of the menu handling perhaps.
For stability reasons, I'll wait until 2.4.0 to include this PR, as I'm already struggling a bit with stability issues re-working a lot of the disk code base. So you would have until the January ISO to do any work on this, if you choose to do so : )

svartkanin · 2021-11-07T11:41:27Z

@Torxed
I've added the simple menu library as a >= dependency so it will always be updated. To keep it more stable we could either lock it to the current version (==) or copy it into the package. I'm generally leaning towards the locking approach since future updates will be a simple version change, however, the external dependency remains. If we copy it into the package, updating will result in bigger diffs.
I'm happy with either option though, so if the goal is to try to keep the package without dependencies then let's copy it over.

I like the idea of having a final version of the options displayed with the possibility to change them again.
To take it a step further, would it make sense to actually have it as a global menu instead? Meaning, instead of going through the single menus sequentially, we could have a root menu where all these options are displayed and can be edited and after all required ones have been chosen the installation can be triggered?

Just to understand, if you would like to wait until 2.4.0 to include it, would we keep it open until then or is it possible to merge it and just not release it? Not sure how the release process works, if it's taking the most recent master it will be tricky I suppose...

Torxed · 2021-11-07T11:50:36Z

@svartkanin Lets copy it over! : ) Reasoning: Bare in mind that all dependencies has to be a Arch Linux package for the inclusion to work, we don't use/avoid relying on pip or similar alternatives to include dependencies. There for, the source has to be included in archinstall or be left omitted I believe.
Perhaps there's some automagic to the way requirements gets pulled upon build, but I highly doubt it.
(keep in mind that this is a Arch Linux package first and foremost, and python library second)

I'm not sure too which scale that would alter the source code, since the code is written around API calls, and guided serves as a decent example of how to build a installer or a templated installation of a system. That would probably mean guided stopped being a decent example and became just another monster like any other distro - making it harder to add or contribute to individual steps. This is just my fear, not something to take for granted.

Yes, we would keep it open (and work on if you wish) until we'ved released v2.3.0 (which is bound to happen really soon), after which we can happily merged this into master if we deem the PR stable (which it kinda is with the option we've got here). If we merge it into master then any new features or changes to this functionality would be a new PR - but until then it's free for all on this branch and we'll merge once stable again : )

profiles/desktop.py

Torxed · 2021-11-25T16:06:58Z

Fixed and merged in this branch & pr's changes to ask_for_main_filesystem_format() with master changes properly. Also fixed some imports and removed the last generic_select() that I could find.

dylanmtaylor · 2021-11-25T16:32:40Z

archinstall/lib/simple_menu.py

+This file is copied over from the simple-term-menu project
+(https://github.com/IngoMeyer441/simple-term-menu)
+In order to comply with installation methods of Arch Linux.
+We here by copy the MIT license attached to the project at the time of copy:


here by -> hereby.

time of copy -> time of copying.

Thank you, I will correct the phrases!

dylanmtaylor · 2021-11-26T16:17:08Z

archinstall/lib/menu.py

+			self.menu_options = [default] + [o for o in self.menu_options if default_option != o]
+
+		cursor = "> "
+		main_menu_cursor_style = ("fg_red", "bold")


Not a huge fan of this color scheme. Maybe we could go with something close to the Arch blue to match?

dylanmtaylor · 2021-11-26T16:18:28Z

archinstall/lib/simple_menu.py

+DEFAULT_CYCLE_CURSOR = True
+DEFAULT_EXIT_ON_SHORTCUT = True
+DEFAULT_MENU_CURSOR = "> "
+DEFAULT_MENU_CURSOR_STYLE = ("fg_red", "bold")


might need changed here as well.

dylanmtaylor · 2021-11-26T16:29:35Z

@svartkanin Could you merge in svartkanin#1? I think the look is a lot better and it makes it match the Arch color scheme.

svartkanin · 2021-11-26T20:53:36Z

@dylanmtaylor That's good, I'll have a look and merge it as soon as I get back to the computer

dylanmtaylor · 2021-11-27T15:41:32Z

I'm wondering how the accessibility of this menu system is with espeak - just saw an issue from a blind user and it made me think of this change

Torxed · 2021-11-27T16:26:37Z

I'm wondering how the accessibility of this menu system is with espeak - just saw an issue from a blind user and it made me think of this change

I've tried it, so far no issues that I can distinguish.
Works well with espeakup at least.

svartkanin · 2021-11-27T22:54:53Z

@dylanmtaylor I left a comment on the PR

dylanmtaylor · 2021-11-29T17:23:52Z

@dylanmtaylor I left a comment on the PR

I suppose it makes sense to not touch the upstream file. I made a commit to remove the change to default menu cursor

dylanmtaylor · 2021-11-29T17:24:22Z

I'm wondering how the accessibility of this menu system is with espeak - just saw an issue from a blind user and it made me think of this change

I've tried it, so far no issues that I can distinguish. Works well with espeakup at least.

My biggest concern is how it would handle conveying which item is selected.

Torxed · 2021-11-29T20:11:36Z

Oddly enough, it works near realtime.
At least when I tried it. It aborts the previous TTS when changing item and simply starts mentioning the new selection.
Something I'm not sure even our current method of menu system does. The TTS also remembers "where you are" without having to re-read all the entries which we currently trigger by wiping the screen (for some of our menus).

Update color scheme to match Arch style better

svartkanin · 2021-11-30T09:37:55Z

@Torxed I saw release 2.3.0 is out, are there any more concerns with this PR or could we consider merging it?
When it's merged I'd be happy to work on the things we mentioned in the comments, foremost to add a global menu after the configuration to re-edit certain settings.

dylanmtaylor · 2021-11-30T22:17:17Z

@Torxed I saw release 2.3.0 is out, are there any more concerns with this PR or could we consider merging it? When it's merged I'd be happy to work on the things we mentioned in the comments, foremost to add a global menu after the configuration to re-edit certain settings.

This is currently targeting 2.4.0, I think it'll be part of the n+2 release, as IIRC a 2.3.1 was planned.

Torxed · 2021-12-01T10:41:29Z

@Torxed I saw release 2.3.0 is out, are there any more concerns with this PR or could we consider merging it? When it's merged I'd be happy to work on the things we mentioned in the comments, foremost to add a global menu after the configuration to re-edit certain settings.

This is currently targeting 2.4.0, I think it'll be part of the n+2 release, as IIRC a 2.3.1 was planned.

Correct. A few things that might need patching. But I could potentially crate a branch of 2.3.0 and cherry-pick certain improvements here and there. I don't think 2.3.1 would be very huge or even relate to v2.4.0. so might be worth doing that, merge this PR into master and all master work goes towards v2.4.0 which will be the larger project?

dylanmtaylor · 2021-12-02T18:08:29Z

@Torxed I saw release 2.3.0 is out, are there any more concerns with this PR or could we consider merging it? When it's merged I'd be happy to work on the things we mentioned in the comments, foremost to add a global menu after the configuration to re-edit certain settings.

This is currently targeting 2.4.0, I think it'll be part of the n+2 release, as IIRC a 2.3.1 was planned.

Correct. A few things that might need patching. But I could potentially crate a branch of 2.3.0 and cherry-pick certain improvements here and there. I don't think 2.3.1 would be very huge or even relate to v2.4.0. so might be worth doing that, merge this PR into master and all master work goes towards v2.4.0 which will be the larger project?

I suppose we could make a branch called future-2.3.1 or something and release a version of that which will ship in Jan, and then use master for 2.4.0 development and merge this PR into master. You'd still be able to cherry-pick into 2.3.1 that way.

Torxed · 2021-12-02T20:18:47Z

Merged, I created v2.3.1-dev for development on the future version v2.3.1 which will contain some minor fixes, and master can continue as usual for "every day development" towards future things to come (v2.4.0 mainly).

Torxed · 2021-12-02T20:19:52Z

@svartkanin Great job on this one, I'm pretty sure it wasn't crystal clear where to change things and replace all the menu options. So thank you for taking the time to dig through the source!

svartkanin · 2021-12-02T20:20:13Z

Sounds good!

svartkanin · 2021-12-02T20:32:21Z

@Torxed it's a really great project and i was very happy to start contributing to it. I'll continue updating things on the menu part but would also like to stqrt picking up other things :)
Is there a project plan or anything the like?

Torxed · 2021-12-02T20:34:30Z

@Torxed it's a really great project and i was very happy to start contributing to it. I'll continue updating things on the menu part but would also like to stqrt picking up other things :) Is there a project plan or anything the like?

I'm not perfect at updating this, but the milestones should broadly represent things we know we want in specific releases: https://github.com/archlinux/archinstall/milestones

I usually do most of the partitioning work because it's a PITA, but feel free to pick up anything you want :) All PR's are welcome, not all make it into master but most of them do.

svartkanin · 2021-12-02T20:37:47Z

Great I'll have a look to see where i can help out

Add simple menu for better UX

deff005

svartkanin requested a review from Torxed as a code owner October 12, 2021 08:22

Torxed requested changes Oct 26, 2021

View reviewed changes

archinstall/lib/user_interaction.py Outdated Show resolved Hide resolved

archinstall/lib/user_interaction.py Outdated Show resolved Hide resolved

archinstall/lib/user_interaction.py Show resolved Hide resolved

archinstall/lib/menu.py Outdated Show resolved Hide resolved

PR feedback

0e7969c

Merge remote-tracking branch 'origin/master' into add-menu

697dce9

Torxed requested changes Nov 7, 2021

View reviewed changes

examples/guided.py Outdated Show resolved Hide resolved

profiles/desktop.py Outdated Show resolved Hide resolved

profiles/i3.py Outdated Show resolved Hide resolved

profiles/server.py Outdated Show resolved Hide resolved

svartkanin added 3 commits November 8, 2021 21:16

Add remove external dependency

bdcf33c

Fix harddisk return value on skip

36ed68b

Table output for partitioning process

5e555f7

Torxed reviewed Nov 8, 2021

View reviewed changes

profiles/desktop.py Outdated Show resolved Hide resolved

svartkanin added 2 commits November 9, 2021 22:50

Switch partitioning to simple menu

bb915a3

fixup! Switch partitioning to simple menu

956b614

Incorrect usage of archinstall. inside the library.

399cfd7

dylanmtaylor reviewed Nov 25, 2021

View reviewed changes

dylanmtaylor reviewed Nov 26, 2021

View reviewed changes

dylanmtaylor added 2 commits November 26, 2021 11:23

Update color scheme to match Arch style better

a5eee0c

Use cyan as default cursor color

9b2252d

Torxed mentioned this pull request Nov 29, 2021

Small UI annoyance when using a prepartitioned layout #776

Closed

Leave simple menu the same

c7194da

Merge pull request #1 from dylanmtaylor/patch-1

fc2e72a

Update color scheme to match Arch style better

Torxed merged commit 908c7b8 into archlinux:master Dec 2, 2021

svartkanin mentioned this pull request Dec 21, 2021

Global menu #806

Merged

Add simple menu for better UX #660

Add simple menu for better UX #660

Conversation

svartkanin commented Oct 12, 2021 • edited by Torxed Loading

Torxed commented Oct 12, 2021

svartkanin commented Oct 12, 2021

mhussaincov93 commented Oct 14, 2021 via email

Torxed commented Oct 14, 2021

svartkanin commented Oct 22, 2021

Torxed commented Oct 26, 2021

svartkanin commented Oct 27, 2021

Torxed commented Oct 27, 2021

svartkanin commented Oct 27, 2021 • edited Loading

Torxed commented Oct 27, 2021

svartkanin commented Nov 1, 2021

Torxed commented Nov 1, 2021

svartkanin commented Nov 1, 2021

Torxed commented Nov 5, 2021 • edited Loading

svartkanin commented Nov 6, 2021

Torxed commented Nov 7, 2021

svartkanin commented Nov 7, 2021

Torxed commented Nov 7, 2021

Torxed commented Nov 25, 2021

dylanmtaylor Nov 25, 2021 • edited Loading

Choose a reason for hiding this comment

Torxed Nov 25, 2021

Choose a reason for hiding this comment

dylanmtaylor Nov 26, 2021

Choose a reason for hiding this comment

dylanmtaylor Nov 26, 2021

Choose a reason for hiding this comment

dylanmtaylor commented Nov 26, 2021

svartkanin commented Nov 26, 2021

dylanmtaylor commented Nov 27, 2021

Torxed commented Nov 27, 2021

svartkanin commented Nov 27, 2021

dylanmtaylor commented Nov 29, 2021

dylanmtaylor commented Nov 29, 2021

Torxed commented Nov 29, 2021

svartkanin commented Nov 30, 2021

dylanmtaylor commented Nov 30, 2021 • edited Loading

Torxed commented Dec 1, 2021

dylanmtaylor commented Dec 2, 2021 • edited Loading

Torxed commented Dec 2, 2021

Torxed commented Dec 2, 2021

svartkanin commented Dec 2, 2021

svartkanin commented Dec 2, 2021

Torxed commented Dec 2, 2021

svartkanin commented Dec 2, 2021

svartkanin commented Oct 12, 2021 •

edited by Torxed

Loading

svartkanin commented Oct 27, 2021 •

edited

Loading

Torxed commented Nov 5, 2021 •

edited

Loading

dylanmtaylor Nov 25, 2021 •

edited

Loading

dylanmtaylor commented Nov 30, 2021 •

edited

Loading

dylanmtaylor commented Dec 2, 2021 •

edited

Loading