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

Implemented StatusController and StatusWriter for setting/updating the status of modules #416

Merged
merged 11 commits into from
Oct 13, 2023

Conversation

rubenthoms
Copy link
Collaborator

@rubenthoms rubenthoms commented Oct 12, 2023

Description

With this PR, modules can report both their loading status as well as per-render errors/warnings which are displayed in the module's respective header. In addition, custom debug messages can be set which are displayed in the DebugProfiler at the bottom of the module's view or settings component, respectively. Moreover, when using this new feature, the module's direct render count (neglecting its sub-components, contrary to the tree render count as viewed by the DebugProfiler itself) is reported to the DebugProfiler and also displayed there.

Finally, added two new components that ought be used when a module's view or settings component runs into an error or does not have anything to show (ContentError or ContentInfo, respectively).


How to use the new feature

In the view component

const view = (props: ModuleFCProps<State>) => {
    ...
    const statusWriter = useViewStatusWriter(props.moduleContext);
    ...
    // Report an error that happens while rendering your component
    if (badCondition) {
        statusWriter.addError("My error message");
    }
    ...
    // Report a warning if something went not as expected but the module is still okay
    if (unexpectedButNotBadCondition) {
        statusWriter.addWarning("My warning message");
    }
    ...
    // Set the module's loading state depending on a condition
    statusWriter.setLoading(myQueries.some(query => query.isFetching))
    ...
    // Set a custom debug message
    statusWriter.setDebugMessage("My custom debug message");
    ...
}

In the settings component

NOTE: The loading state can currently only be set with the ViewStatusWriter.

const settings = (props: ModuleFCProps<State>) => {
    ...
    const statusWriter = useSettingsStatusWriter(props.moduleContext);
    ...
    // Report an error that happens while rendering your component
    if (badCondition) {
        statusWriter.addError("My error message");
    }
    ...
    // Report a warning if something went not as expected but the module is still okay
    if (unexpectedButNotBadCondition) {
        statusWriter.addWarning("My warning message");
    }
    ...
    // Set a custom debug message
    statusWriter.setDebugMessage("My custom debug message");
    ...
}

Closes #375.

@rubenthoms rubenthoms added the enhancement New feature or request label Oct 12, 2023
@rubenthoms rubenthoms self-assigned this Oct 12, 2023
Copy link
Collaborator

@sigurdp sigurdp left a comment

Choose a reason for hiding this comment

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

Added some minor suggestions, and some thoughts on an alternative approach to the division of responsibilities between ModuleInstanceStatusController and ModuleInstanceStatusControllerPrivate

Let's have a discussion before proceeding on this.

frontend/src/framework/StatusWriter.tsx Outdated Show resolved Hide resolved
frontend/src/framework/StatusWriter.tsx Outdated Show resolved Hide resolved
frontend/src/framework/ModuleInstanceStatusController.ts Outdated Show resolved Hide resolved
frontend/src/framework/StatusWriter.tsx Outdated Show resolved Hide resolved
@rubenthoms rubenthoms requested a review from sigurdp October 13, 2023 08:53
Copy link
Collaborator

@jorgenherje jorgenherje left a comment

Choose a reason for hiding this comment

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

Tested and I really like the features! With respect to code, Sigurd have written a good feedback there.

Looks good to me, only one question (not needed to be changed)

Question regarding the render of settings error/warnings: These are placed in the header of the module view, is it wanted to have one single place to show these, or could settingsStatusWriter render in the settings header? (I know the settings status will disappear when other module settings or config is rendered in the drawer)


Here is a screenshot of testing "invalid" selected vector in settings.

image

Copy link
Collaborator

@sigurdp sigurdp left a comment

Choose a reason for hiding this comment

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

Good to go 👍

@rubenthoms rubenthoms merged commit 120053c into equinor:main Oct 13, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create visualization of query error and fetching status for modules
3 participants