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

fix(<stencila-ui-node-execution-messages>): improve rendering of messages as per designs #2127

Closed
nokome opened this issue Mar 14, 2024 · 5 comments · Fixed by #2129
Closed
Assignees
Labels
module: web Related to web browser clients and interfaces type: bug Addresses a bug

Comments

@nokome
Copy link
Member

nokome commented Mar 14, 2024

Backgound

All node types derived from Executable have an executionMessages property. We need to fix the rendering of these so that they match the designs closer. Currently they look like this:

image

The designs look like this:

image

Tasks

  • Create a <stencila-ui-node-execution-messages> component, consistent with out approach of creating property-level components. This component will be the container for one or messages and have a counter for the number of messages at each level.
  • Create a <stencila-ui-node-execution-message> to represent each individual ExecutionMessage with collapsable stack trace etc

@mike-parkin I'm going to setup the new components in a new PR and then hand it over to you.

@nokome nokome added type: bug Addresses a bug module: web Related to web browser clients and interfaces labels Mar 14, 2024
@mike-parkin
Copy link
Collaborator

@simonwinter is this related to what you're doing at the moment? might need to hold off implementing this issue until that is wrapped up.

@simonwinter
Copy link
Collaborator

@mike-parkin - yeah, it's definitely related. to #2124. Let's get that merged first.

@simonwinter
Copy link
Collaborator

simonwinter commented Mar 14, 2024

@mike-parkin when you get around to this, the pattern we've used to render the execution-details (src/ui/nodes/properties/execution-details.ts) will probably be similar to what you need to do here, at least from the outside-in:

<stencila-collapsible-node-field
  .collapsed=${false}
  headerBg=${this.headerBg}
>
  <div slot="title">Messages</div>
  <div class="px-6 py-3 flex flex-col gap-y-3" slot="content">
    ...
  </div>
</stencila-collapsible-node-field>

I'd like to re-think the headerBG - this may be some style object we get from #2126.

In my mind, I see this as a repeatable pattern ... though the use of gap-y-3 to handle margin between elements may not be as useful for you 🤷. Anyway, the point is that when you get to this, let's review what we can/can't repeat. There are some savings to be made here :)

@nokome
Copy link
Member Author

nokome commented Mar 14, 2024

@mike-parkin I've closed the wip PR so you can start this on top of the work @simonwinter has done. Can you please start a new PR off main.

@nokome
Copy link
Member Author

nokome commented Mar 14, 2024

I'd like to re-think the headerBG - this may be some style object we get from #2126.

I suggest that we use the pattern that we have established with <stencila-ui-node-authors> where we pass the NodeType as an attribute (named type to avoid clash with HTMLElement's nodeType property):

image

Then whichever components are in the hierarchy can use that to fetch whichever colours or icons they need from a function in icons-and-colours.ts. This is what <stencila-ui-node-authors> does:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: web Related to web browser clients and interfaces type: bug Addresses a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants