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

Clarify that setting a static IP only applies to Ethernet, not WiFi #1815

Open
jotaen4tinypilot opened this issue Jul 3, 2024 · 5 comments
Assignees

Comments

@jotaen4tinypilot
Copy link
Contributor

Related #131. (Originally subtask (e)).

Setting a static IP address for the device only applies to the Ethernet interface, but it doesn’t have an effect on WiFi. This is not obvious to users, so we should clarify this e.g. in the static IP dialog.

@jotaen4tinypilot
Copy link
Contributor Author

I’ve split out this ticket from the original one, so that we can discuss this topic separately.

The most straightforward solution here would be to expand the intro text in the static IP dialog, to point out the Ethernet limitation:

Screenshot 2024-07-03 at 18 52 32

This may make the intro text rather long, though. We could also consider making the previous intro more concise, to fit the new addendum a little better:

Screenshot 2024-07-03 at 19 02 30

@shalver-tp Do you have a preference here, or other thoughts?

@shalver-tp
Copy link

@jotaen4tinypilot

I think trying to make it more concise is valuable.

I'd also think about making the "Note" section called out in a different color. You could Then simplify that text to "Setting a static IP only affects the Ethernet connection."

I think there are some places in the UI that use a different color or box to call out (see "Security Settings").

I'd love @cghague to weigh in also when he has a chance.

@jotaen4tinypilot
Copy link
Contributor Author

We use orange message boxes in some places, e.g. in the Security Dialog. That could look like so:

Screenshot 2024-07-03 at 20 13 24

The orange message boxes are currently reserved for “warning”-type messages – see our style guide (rendered version of the section).

I’m not sure a “warning” is applicable here, though, because the note in this case is not really actionable for the user, but it’s more informational, and educates the user about certain limitations.

We could consider adding a third “info”-type message box, that’s a bit more subtle and less “alarmist”.

Screenshot 2024-07-03 at 20 14 08

We don’t have that style variant of the message box yet, but we could easily add it.

I think in the end the question is how “bold” and prominent we want this note to be. My take would be:

  • If the note is only subsidiary, it might be best off as regular text, without special styling.
  • If we feel that it’s somewhat critical for the user to not miss this note, then we could introduce an “info”-type message box.

@shalver-tp
Copy link

@jotaen4tinypilot I agree the orange/red is too harsh.

I'm not completely averse to adding a new style, but I'll wait for Charles' take on how critical this note might be.

@cghague
Copy link
Contributor

cghague commented Jul 3, 2024

@shalver-tp / @jotaen4tinypilot - We already use the blue callouts in some of our frequently asked questions, and I think we'd benefit from extending that pattern into the app as well. We should use it sparingly, but I think it's a good choice for this particular scenario in the first instance.

jotaen4tinypilot added a commit that referenced this issue Jul 5, 2024
As discussed in #1815, we
want to add an informational message box to the Static IP dialog (in
Pro).

This PR introduces the `info`-type variant for our `<inline-message>`
component, and defines its usage in our app styleguide.

Similar to [the existing
`--brand-metallic-lighter`](https://github.com/tiny-pilot/tinypilot/blob/04108124805238b1554c588fab8e6a3a0c7817d6/app/static/css/style.css#L17),
I had to introduce a new blue colour variant, otherwise the background
of the box would have been too dark (when using `--brand-blue-light`).

<img width="732" alt="Screenshot 2024-07-04 at 12 32 03"
src="https://github.com/tiny-pilot/tinypilot/assets/83721279/52ad11f5-e708-4fa2-bfc3-a6b42a5b5fe0">

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1818"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

---------

Co-authored-by: Jan Heuermann <[email protected]>
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

No branches or pull requests

3 participants