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

Take two: distinguish nodes in DAG viz based on access #398

Merged
merged 5 commits into from
Mar 15, 2023

Conversation

jtcohen6
Copy link
Contributor

follow-up to #378 & #389

Description

Got some quick design feedback from @amy-byrum on the approach we're taking to visually distinguishing models based on their access modifier (public/private/protected).

We're very supportive of keeping the default display for the default access (protected). I think, rather than changing the shape of the node, let's use opacity/border to show:

  • More transparent nodes for private
  • More solid nodes for public

Definitely open to feedback here! This is why we beta early & often :)

Screenshot 2023-03-12 at 19 25 26

Screenshot 2023-03-12 at 19 25 32

Screenshot 2023-03-12 at 19 25 40

Screenshot 2023-03-12 at 19 26 51

Checklist

Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

This feels like an improvement, but still not completely intuitive. Visually separating three different categories is hard! (Especially when color is already used for other purposes.)

TL;DR

I wouldn't complain if this was the final design we land on -- it looks good and separates the three different modifiers (even if it's not completely obvious what each represents).

Things I like

  • default of access modifier of protected stays the same visually
  • letting go of the oval shape :)

Things that aren't fully intuitive to me

  • what each of the different background and borders mean
    • transparent background feels more "open" to me (which feels more "public")
    • but on the other hand, may transparent also feels "unknown" since it can't be fully seen 🤷

An idea to consider:

  • would the rectangle shape be useful to reintroduce?
    • Maybe to delineate private nodes? It feels closed off as a shape.

Shapes

Before:

After:
Screenshot 2023-03-12 at 19 25 26

@amy-byrum
Copy link

@jtcohen6 I thought we were only distinguishing 2 access levels vs all three? Did I misunderstand?
Either way--this is definitely an improvement from take one! Some feedback from me:

Given these three states:

  • fill + no stroke
  • fill + stroke
  • no fill + stroke

The former 2 are not as easy to scan/differentiate. In other words, on the nodes that have a fill, my eyes want to just ignore the border. This may be in part a contrast issue (the white and blue being used for model notes do not have adequate contrast in accordance with WCAG). Playing with the stroke weight may help, and/or making some minor adjustments to the blues we're using. If you're willing to tweak those I can help with some suggestions. I'm not implying anything drastic--we could increase legibility with some hex changes that would hardly be noticeable to most users. Quick and dirty example attached!
Screenshot 2023-03-13 at 3 08 50 PM

I'm also a bit confused w/ what's happening on the vertical view--the top and bottom nodes both appear to be fill + stroke, but the stroke is different? Am I reading that incorrectly?

Lastly--as you know I share Doug's desire for a better way to let users find out what the visual differences mean as it's not intuitive, but I don't think it should be a blocker to an mvp state. It's about as intuitive as green = source, I reckon. :)

@jtcohen6
Copy link
Contributor Author

@dbeatty10 @amy-byrum Thank you for the awesome feedback!!

Here's what I'm thinking:

  • Let's trim down the set of visually distinct access levels from three (protected/default, public, private) to two (default vs. private). The distinction between protected and public isn't functionally meaningful until we support cross-project references. (Amy, you understood right when we chatted last week: While we have three access levels, our conversation last week was more focused on the binary distinction between "private" and "not private.")
  • It will be easier to add further demarcation with icons, which is what the newer DAG viz in the dbt Cloud IDE aims to do. In the meantime, the visual distinction between "fill" and "no fill + stroke" is clear enough to spot, and something for which we can document the meaning.
  • The more compelling version of this visual would also enable us to demarcate model groups. That requires a bigger lift than what we're capable of here. I'd rather make a smaller change for now, than a bigger change that will actually appear less complete for lack of complementary pieces.

@jtcohen6 jtcohen6 force-pushed the jerco/design-review-node-access branch from 7e7103d to 025d50d Compare March 14, 2023 23:58
@jtcohen6
Copy link
Contributor Author

I'm going to merge this in, so that we can include it in the next beta (Thursday). I'll kick off the automated PR workflow after merging.

cc @MichelleArk @emmyoop

@jtcohen6 jtcohen6 merged commit 7e2871a into main Mar 15, 2023
@jtcohen6 jtcohen6 deleted the jerco/design-review-node-access branch March 15, 2023 11:46
jtcohen6 added a commit that referenced this pull request Mar 15, 2023
jtcohen6 added a commit that referenced this pull request Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants