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

Display warnings on components #9108

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

MichaelMauderer
Copy link
Contributor

@MichaelMauderer MichaelMauderer commented Feb 20, 2024

Pull Request Description

Closes #8678

Peek.2024-02-20.12-45.webm

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@MichaelMauderer MichaelMauderer added CI: No changelog needed Do not require a changelog entry for this PR. -gui labels Feb 20, 2024
@MichaelMauderer MichaelMauderer self-assigned this Feb 20, 2024
@MichaelMauderer MichaelMauderer force-pushed the wip/MichaelMauderer/Display-warnings-on-components branch from b54c89f to 22908a2 Compare February 20, 2024 13:58
@MichaelMauderer MichaelMauderer marked this pull request as ready for review February 20, 2024 14:23
<GraphNodeError v-if="error" class="error" :error="error" />
<GraphNodeError v-if="error" class="message" :message="error" type="error" />
<GraphNodeError
v-if="warning && nodeHovered && isSelected"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised by nodeHovered && isSelected here, are our conditions for showing warnings that restrictive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what was requested by @AdRiley

Only show when node is selected and when hovering over it.

Or was that meant to mean, we show it when selected OR when hovering it?

Copy link
Member

Choose a reason for hiding this comment

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

Yes it should be OR. I should have written that better sorry.

app/gui2/src/components/GraphEditor/GraphNodeMessage.vue Outdated Show resolved Hide resolved
if (!externalId) return
const info = projectStore.computedValueRegistry.db.get(externalId)
const warning = info?.payload.type === 'Value' ? info.payload.warnings?.value : undefined
if (!warning) return undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

I think return or returned undefined are both fine but let's not mix styles within one function

@MichaelMauderer MichaelMauderer added the CI: Ready to merge This PR is eligible for automatic merge label Feb 21, 2024
@mergify mergify bot merged commit 6c4c791 into develop Feb 21, 2024
26 checks passed
@mergify mergify bot deleted the wip/MichaelMauderer/Display-warnings-on-components branch February 21, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display warnings on components
3 participants