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 error display to core widgets. #2129

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

dkbennett
Copy link
Member

Summary of the pull request

Core widgets currently do not set ContentData to anything on error, which causes them to display an empty template with variable names with no indication of what is wrong. This change adds an error message to the main template for each of the core widgets, and populates that message with any exception errors when encountered so it is obvious to the user that something is wrong. If there is no error message, then the templates display as normal, but if there is an error message that is displayed instead.
 

References and relevant issues

Closes #1248

Detailed description of the pull request / Additional comments

  • Each core widget now sets an error message when an exception occurs during LoadContentData.
  • Each core widget template has a container for displaying any error messages that is mutually exclusive with its normal display (if error, show error, else show normal template).

The diff of the templates looks like a lot changed, but the actual change was adding a new container for the error message and then shifting the previous content to be in a second container mutually exclusive with the error container. Shifting it one indentation level made for a messy JSON diff.

Validation steps performed

  • Manual test by forcing failure in the LoadContentData method of each widget.
  • Confirmed that the widgets look normal without failure and that they show the failure message with
  • Exception: I was unable to validate the SSH widget as I had no SSH config file.

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@dkbennett dkbennett added the Needs-Second Pull request that needs another approval label Jan 12, 2024
@@ -86,6 +86,11 @@ public override void LoadContentData()
catch (Exception e)
{
Log.Logger()?.ReportError(Name, ShortId, "Error retrieving data.", e);
var content = new JsonObject
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider promoting error message object creation to a method in the parent class (reusability + maintenance)

@EricJohnson327 EricJohnson327 merged commit b569886 into main Jan 18, 2024
4 checks passed
@EricJohnson327 EricJohnson327 deleted the user/dkbennett/corewidgetnocounters branch January 18, 2024 00:49
@krschau krschau removed Needs-Second Pull request that needs another approval Area-Widgets Related to in-package widgets labels Jan 25, 2024
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.

Core widgets showing variable names
5 participants