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

add Init Status and Variant #158

Merged
merged 1 commit into from
Oct 30, 2023
Merged

add Init Status and Variant #158

merged 1 commit into from
Oct 30, 2023

Conversation

jj-so
Copy link
Contributor

@jj-so jj-so commented Oct 20, 2023

  • check the device status in the OverviewTab and shows the Init status and also the variant.
  • If the init status is not "ok" a button with a warning sign and a link to support is displayed.

Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me and also works when the device sends non-zero status values. Some thoughts regarding the UI:

  • What do you think about moving the warning icon before the status value so that it is clear what it refers to?
  • If we link to the documentation, the button should probably say something like “More Info” instead of “Contact Support”.
  • The link to the documentation on the home screen is just a regular text link, not a push button. I’d prefer to also use that format here.
  • It would be good to have a tooltip when hovering the icon or the version value in the error case, for example: “An error occurred during device initialization. Click the ‘More Info’ button for more information and contact support if the error persists.”

We can discuss the UI details on Tuesday.

nitrokeyapp/overview_tab.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@daringer daringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm,

  • please squash the commits together into 1 or 2 with proper commit messages
  • I resolved the review-requests from @robin-nitrokey already

@jj-so jj-so merged commit 697677b into main Oct 30, 2023
8 checks passed
@jj-so jj-so deleted the device_status branch October 30, 2023 13:59
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.

Check device status after connection and display in overview screen
3 participants