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

UX experiment #113

Closed
wants to merge 3 commits into from
Closed

UX experiment #113

wants to merge 3 commits into from

Conversation

bugadani
Copy link
Contributor

No description provided.

@bjoernQ
Copy link
Collaborator

bjoernQ commented Feb 12, 2025

definitely adds some complexity and might take a user some time to understand

what we could do it change the foreground color of a highlighted but available option - i.e.

image

this looks like the first option in disabled and it's the initial state when entering that sub-menu

other than that I think the main source of confusion is having relations between different screens (rtt on the main level vs logging and panic-handler) - but I don't see how to fix that without changing the whole UI-concept (into not having sub-screens and having dependent options "somehow under" the item they are dependent on .... not sure how to phrase that - or maybe keep sub-screens for things which are not related to anything else 🤔 )

@bugadani
Copy link
Contributor Author

bugadani commented Feb 12, 2025

Thanks! Is it better or worse if probe-rs shows up in two places?

For me, probably not much better. How about if everything is grouped into one screen 🤔

@bjoernQ
Copy link
Collaborator

bjoernQ commented Feb 12, 2025

I think now with the submenu it's better

Maybe we shouldn't offer defmt via esp-println - that might make things a bit easier?
Too many choices might confuse users (and the UI)

@MabezDev
Copy link
Member

I struggled a bit with this to be honest 😅. I think I would organize it like:

  • espflash (selected by default)
    • ... list the possible front/back ends for logging whilst using espflash
    • ... list the supported panic handlers
  • probe-rs (if selected would disable espflash and hide submenu above)
    • ... list the possible front/back ends for logging whilst using probe-rs
    • list the supported panic handlers

Thoughts? (if that's even possible)

@bugadani
Copy link
Contributor Author

bugadani commented Feb 12, 2025

Thoughts? (if that's even possible)

It's certainly possible, just not with the current software. We can't have default-on options, because the -o flags can't disable them currently. We could allow -o !espflash I guess, but I'm not sure how that will affect testing the combinations.

We also don't hide items currently. We could hide empty categories (as in, without active options), but that means they are hidden until you find out which option opens the door, while currently you can move over one of the items and we print what is required for it to become active.

Maybe we shouldn't offer defmt via esp-println - that might make things a bit easier?

Ironically, that is the option that needs the most hand-holding. We can hide it behind an advanced template, along with probe-rs and the RTT-related options, I think. We could use the same template project, just a different template yaml. The downside is that we can't list the template options via --help because there are multiple sets to choose from.


I guess I can figure out hiding options, that way we can probably have more advanced stuff, as well as separate probe-rs/espflash settings.

@bjoernQ
Copy link
Collaborator

bjoernQ commented Feb 13, 2025

I think with the separate sub-sections it's much better

@bjoernQ
Copy link
Collaborator

bjoernQ commented Feb 13, 2025

I think these colors look a bit better

const SELECTED_ACTIVE_BACKGROUND: Color = tailwind::BLUE.c950;
const SELECTED_INACTIVE_BACKGROUND: Color = tailwind::GRAY.c700;

const SELECTED_ACTIVE_STYLE: Style = Style::new()
    .add_modifier(Modifier::BOLD)
    .fg(TEXT_COLOR)
    .bg(SELECTED_ACTIVE_BACKGROUND);

image

currently looks like this
image

Makes it imho more obvious the first option is selectable

@bugadani
Copy link
Contributor Author

Definitely better, but not exactly the point of this PR :) I'll submit the ability to set these colors and the styling itself separately.

@bjoernQ
Copy link
Collaborator

bjoernQ commented Feb 13, 2025

not exactly the point of this PR

it's also about UX - but sure another PR is fine

@bugadani bugadani force-pushed the granular-deps branch 2 times, most recently from a028a1a to 09dcf96 Compare February 13, 2025 12:01
@bugadani
Copy link
Contributor Author

bugadani commented Feb 13, 2025

This is now rebased on top of #119. One bug I see is that enabling defmt in espflash also enables it in probe-rs, which is technically true but visually confusing.

@bugadani
Copy link
Contributor Author

I've changed probe-rs to display a different help text for devices without usb-jtag (and c3, which I still don't know definitely whether it is borked or not). This needs some logic change, but I think it's an interesting possibility to provide chip-specific information.

@bugadani bugadani closed this Feb 13, 2025
@bugadani bugadani deleted the granular-deps branch February 13, 2025 15:50
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