-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Update logic graph styling #2912
Conversation
Removed vultr server and associated DNS entries |
f359e7d
to
3b1472d
Compare
1e6b5a8
to
88c3def
Compare
88c3def
to
50d0d12
Compare
@ianjon3s Destroyed and recreated the Vultr instance here this morning as I noticed it was failing. Let us know when this is ready for review as I know you're keen to move it forward 👍 |
Thanks @DafyddLlyr ! I've marked this as ready-to-review as I believe it is in good shape from a code perspective. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nice refactor in future would be to replace this with styled components to match the rest of the codebase. Not a small task and no rush really, but it would be nice from a consistency point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I'll see about doing this as a longer-term piece of work.
className={classNames( | ||
"card", | ||
"decision", | ||
"type-" + TYPES[props.type as TYPES], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha! I forgot about this neat little trick.
I think this means we need to update the "how to add a component" guide to say we need to add a style for a new component also (not sure how this would handle defaults / new components right now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case the style is only being used/applied as an enhancement for type-Section
, so we wouldn't need a style unless it needed to be distinctive from the other components. As long as the type is set on a new component it'll pull through and be used if necessary (if I understand your point correctly).
Looking great! |
What does this PR do?
Visual changes to the logic graph, summarised as:
Preview
Before:
After: