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

Apply diagram changes #175

Closed
iherman opened this issue Aug 22, 2023 · 25 comments
Closed

Apply diagram changes #175

iherman opened this issue Aug 22, 2023 · 25 comments
Assignees
Labels
CR1 This item was processed during the first Candidate Recommendation phase. pending close (7 days) This issue will be closed after 7 days.

Comments

@iherman
Copy link
Member

iherman commented Aug 22, 2023

This is just a warning place to adopt some changes on the vocabulary diagrams, see #171 (comment) and the followups in #171 (comment) and #171 (comment)

@iherman iherman added the CR1 This item was processed during the first Candidate Recommendation phase. label Aug 22, 2023
@iherman iherman self-assigned this Aug 22, 2023
@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

@TallTed, but also @dlongley, @msporny, @seabass-labrax

I have updated the two diagrams and put them up into the yml2vocab previews. I prefer not to touch the "real" files on the vcdm, resp. di, repositories; real PRs can come later when the last minute pre-cr frenzy is over. But I would welcome your comments, so that the final incorporation would only be a formality.

The two previews are here:

@TallTed I believe I have incorporated your comments, but your check would be welcome.

I have checked the color choices to see if it works all right with dark mode as well. I tested the files on several chromium based browsers with the experimental flag forcing dark mode on web sites; I also tested it with the "Dark Reader" plugin on Firefox. In all cases it looked o.k. to me, but @dlongley should be the final arbiter.

I actually had a version (which is mostly the version in the "official" drafts without the changes as requested by @TallTed) that did not use any background colors in the shapes. I kept those versions as well, but I personally prefer the more colorful version. It is of course a matter of taste; if you think it is better not to use colors to fill the shapes, I am happy to switch back.

(I am out next week, so no rush... I just wanted to get it out of the door before I go away...)

@seabass-labrax, just for info the process now is : export from draw.io and then run it through a script that is built on top of SVGO...

@msporny
Copy link
Member

msporny commented Aug 25, 2023

I would welcome your comments, so that the final incorporation would only be a formality.

The diagrams are fantastic, thank you @iherman!

The only thing that jumped out at me was this:

image

Why is there an "OR" there? What does it mean?

@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

You are good at picking the weak points, @msporny.

These three properties are special in the sense as their domain is VerifiableCredential OR VerifiablePresentation, and I did not find a better way to represent this fact.

Some (weak) alternatives:

  • Create two red arrows to each of those, coming from, respectively, the two nodes representing the two classes. The result is a bit messy with the three pairs of arrows crossing.
  • Place the three rectangles representing the properties horizontally between the two class nodes and connect them with the arrows (ie, each property would receive a node from above and from under)
  • Like now, but remove the OR node and just have a dot where the arrows meet.

None of these are good alternatives...

@TallTed
Copy link
Member

TallTed commented Aug 25, 2023

@iherman — The revision does some things as I had requested, but I appear to have been unclear about others.

https://w3c.github.io/yml2vocab/previews/vcdm/vocabulary.svg

  1. The direction of an arrow generally gives the way to read that arc, e.g., VerifiablePresentation rdfs:domain holder and VerifiablePresentation rdfs:domain verifiableCredential -- but these are obviously backwards. Most of those now reading backwards previously had diamonds at the ends which are now the back of the arrows. I had tried to suggest changing the diamonds to arrowheads, which I had meant to point the opposite direction from what is now present. Maybe that's now clearer?

  2. As @msporny did, I noticed the OR circle. I think that's trying to say that "VerifiablePresentation OR VerifiableCredential could be Domain (a/k/a schema:domainIncludes VerifiablePresentation, VerifiableCredential) of termsOfUse, validFrom, validUntil" — but I don't understand why this would only apply to those three properties. To my mind (and I hope the text and all bear this out), most if not all pictured properties should have schema:domainIncludes VerifiablePresentation, VerifiableCredential. In any case, I would make all these arrows complete arcs, and do away with the OR as well as the vertical connectors now seen between VerifiablePresentation, OR, and VerifiableCredential.

  3. The tiny-dots and tiny-spacing of the Domain arrows are TOO tiny. Better to make these longer dashes with bigger gaps than the current dashed lines of the Range arrows.

  4. I would remove the contains label from that arrow, as it's now part of the key, and I would change Graph containment in the key to simply Contains, to match the other labels in the key.

  5. I urge checking all contrast ratios between text and background colors, in both light mode and dark mode. This is key for accessibility review. I don't think they pay much attention to red-green or blue-yellow colorblindness concerns, but it would be good form to also avoid those, if possible.

  6. Is JsonSchema Superclass of CredentialSchema, or is CredentialSchema Superclass of JsonSchema?

There are additional questions raised by the other diagram, but I think it probably better to get this one fixed, apply the fixes that apply to both to the other, and then dig further into the other.

@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

@TallTed I will take care of the changes in time, just two remarks:

  • For the OR node: the diagram reflects what is in the vocabulary which, on its turn, reflects the spec at least the way I understand it. The 'issuer', 'evidence', etc, properties are only mentioned in conjunction with a credential and not with a presentation. I am not the domain expert here, any change should come from the VCDM spec.
  • Thanks for the contrast ratio checker pointer, I was looking for something like that. I am a bit worried that the double constraint of contrast ratio and dark mode will make the colored version of the diagram impossible, though. The way the dark modes are generated is a bit of a mystery to me and different from one browser, or from one plugin to the other (let alone the fact that, on Chromium at least, there are 5-6 alternatives to choose from for setting the dark mode...)

@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

One more remark, @TallTed: per the vocabulary, CredentialSchema is a superclass of JsonSchema. Again, if that is incorrect, the vocabulary should be changed and the spec clearer...

@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

Actually...

The direction of an arrow generally gives the way to read that arc, e.g., VerifiablePresentation rdfs:domain holder and VerifiablePresentation rdfs:domain verifiableCredential -- but these are obviously backwards

This may not be what you originally had in mind, but I am not sure it is really wrong as is. The current representation suggests that information "flows" from a class to another one via the property and, although I know this is not what a property does, it somehow fits the mental model for a domain and the range as far as I am concerned...

@dlongley
Copy link
Contributor

@iherman,

Here's what the VCDM diagram looks like to me:

dark-mode-vcdm-diagram

The borders are a little hard to make out.

@dlongley
Copy link
Contributor

Here's what the data integrity diagram looks like to me:

darkmode-di-diagram

@iherman
Copy link
Member Author

iherman commented Aug 25, 2023

@dlongley yes, that is in line with what I see. And, actually, just looking at the image makes it clear that the diagram would not pass the contrast ratio tests.

At this point, I am tempted to say that we should abandon my attempt to use background colors for the shape. It just not possible to solve the equation with contrast ratios, dark and light modes, and the unclear algorithms the browsers use to turn an SVG file prepared for light mode into dark mode.

We have other issues to discuss on the content; for the visual appearance we should keep to what is, for example, in the current version of the vocabulary file.

WDYT?

(Edited; see #175 (comment).)

@TallTed
Copy link
Member

TallTed commented Aug 25, 2023

  • For the OR node: the diagram reflects what is in the vocabulary which, on its turn, reflects the spec at least the way I understand it. The 'issuer', 'evidence', etc, properties are only mentioned in conjunction with a credential and not with a presentation. I am not the domain expert here, any change should come from the VCDM spec.

Ayie! It's doubly good we have these diagrams now, and that they were generated based-on/from the vocabs, which were generated based-on/from the spec — because this shows that the specs and the vocabs are wrong!

  • Thanks for the contrast ratio checker pointer, I was looking for something like that. I am a bit worried that the double constraint of contrast ratio and dark mode will make the colored version of the diagram impossible, though. The way the dark modes are generated is a bit of a mystery to me and different from one browser, or from one plugin to the other (let alone the fact that, on Chromium at least, there are 5-6 alternatives to choose from for setting the dark mode...)

Yeah, dark mode is a dark art. As you've found, it can vary within a single app on a single OS. It also varies between different apps on one OS, and even more across different OS. Some do a partial color inversion; some do a full inversion; and others perform other tricks.

All of which means we can only really control the light mode, and test dark mode on whatever OS and/or browsers we each have, toward making some recommendations to readers for what settings to apply when reading our documents.

The direction of an arrow generally gives the way to read that arc, e.g., VerifiablePresentation rdfs:domain holder and VerifiablePresentation rdfs:domain verifiableCredential -- but these are obviously backwards

This may not be what you originally had in mind, but I am not sure it is really wrong as is. The current representation suggests that information "flows" from a class to another one via the property and, although I know this is not what a property does, it somehow fits the mental model for a domain and the range as far as I am concerned...

Properties have domain and range, which are either Classes or Datatypes.

Classes have neither domain nor range; they have Properties.

I have no argument with the perceived suggestion of information "flow," but if that's what's being diagrammed, it should be labeled as such.

@iherman
Copy link
Member Author

iherman commented Aug 26, 2023

  • For the OR node: the diagram reflects what is in the vocabulary which, on its turn, reflects the spec at least the way I understand it. The 'issuer', 'evidence', etc, properties are only mentioned in conjunction with a credential and not with a presentation. I am not the domain expert here, any change should come from the VCDM spec.

Ayie! It's doubly good we have these diagrams now, and that they were generated based-on/from the vocabs, which were generated based-on/from the spec — because this shows that the specs and the vocabs are wrong!

Whether it is wrong or not, I do not know. I am in the humble position of translating the spec into the RDF world, and I do not claim to be a domain expert for VCs and VPs.

@TallTed I would prefer not to discuss this problem in this issue. Would you mind raising a separate issue in the VCDM repo and possibly discuss it there?

@iherman
Copy link
Member Author

iherman commented Aug 26, 2023

The direction of an arrow generally gives the way to read that arc, e.g., VerifiablePresentation rdfs:domain holder and VerifiablePresentation rdfs:domain verifiableCredential -- but these are obviously backwards

This may not be what you originally had in mind, but I am not sure it is really wrong as is. The current representation suggests that information "flows" from a class to another one via the property and, although I know this is not what a property does, it somehow fits the mental model for a domain and the range as far as I am concerned...

Properties have domain and range, which are either Classes or Datatypes.

Classes have neither domain nor range; they have Properties.

There is one more, purely aesthetic, reason why I would prefer to keep the diagram as is in this respect. If I have a bunch of lines all having the same starting point (on the Class) and all having an arrowhead, the result will be a big blob (unless all the lines are strictly vertical at that point but that means the connection lines would take up more real estate).

I have no argument with the perceived suggestion of information "flow," but if that's what's being diagrammed, it should be labeled as such.

Would it be a solution to keep the diagram as is in this respect and label it as "Domain of" ?

@iherman
Copy link
Member Author

iherman commented Aug 26, 2023

For those who have not experienced the blak art of dark mode... the example of @dlongley in #175 (comment) is based on the experimental setting of chromium to enforce dark mode on every site. I have installed the "Dark Reader" extension to my Chromium based browser instead, yielding the following figure for the same diagram:

Screenshot 2023-08-26 at 08 32 31

This is really black art... Although, I must admit, the extension seems to make a better job at least for my eyes...

(The extension is also available for firefox and safari, although, interestingly, it is a paying extension for the latter.)

@iherman
Copy link
Member Author

iherman commented Aug 26, 2023

To my (pleasant) surprise, using the contrast tool referred to by @TallTed, it was relatively easy to modify the colors (namely the font colors) to make the diagram pass the contrast checks and, at least on my machines, produce a proper dark mode image as well. (It looks perfectly fine when using the DarkReader plugin, mostly like #175 (comment), at also ok with the default Chromium built-in flag).

It seems that I was pessimistic in #175 (comment). @dlongley, if it is also acceptable for you, then we could consider that part of the changes done. WDYT?

@dlongley
Copy link
Contributor

@iherman,

That looks ok to me, but I would be concerned about having red text on a green background... my non-expert thought is that those things would be hard to read for people with deuteranopia.

@iherman
Copy link
Member Author

iherman commented Aug 27, 2023

That looks ok to me, but I would be concerned about having red text on a green background... my non-expert thought is that those things would be hard to read for people with deuteranopia.

Maybe. But this is where I am completely out of control. The BW (original) version of the text passes the contrast control, the default chrome action for dark mode creates something different than the one you refer to that is generated by a particular browser extension. There is no standard in the area, and there is no way we could test with all available extensions… More importantly, there is no control over how things look like in dark mode.(*)

I would propose to keep this as is for now. We will see whether there is an issue in practice; there is always the fall-back of ignoring all colors. WDYT?

(*) there is, in theory: do the whole diagram in SVG directly, and by hand. That would mean adding (CSS) class identifiers to all shapes, and use CSS for the color settings. Then use the dark mode media query to set these colors the right way. However, doing the diagram entirely by hand is not something I am prepared to do; the only tool that works with SVG out of the box (that I know of) is Inkscape which, alas!, has a bunch of bugs in the connecting lines which make it fairly unusable for these types of diagrams. ☹️

@dlongley
Copy link
Contributor

I would propose to keep this as is for now. We will see whether there is an issue in practice; there is always the fall-back of ignoring all colors. WDYT?

+1, sounds good to me.

@TallTed
Copy link
Member

TallTed commented Aug 28, 2023

[@iherman] Would it be a solution to keep the diagram as is in this respect and label it as "Domain of" ?

Yes.

@iherman
Copy link
Member Author

iherman commented Aug 29, 2023

I have refreshed the diagrams, trying to incorporate all the comments of @TallTed.

(I am out for the rest of the week)

@TallTed
Copy link
Member

TallTed commented Aug 29, 2023

@iherman -- Just one more thing, when you return, presuming that the domain/range statements and illustrative connectors remain as they are.

The point where 5 lines come together, between VerifiablePresentation and verifiableCredential — I would like to move this to the right, to about halfway between its current position and the left edges of the property boxes. An arrow head (or maybe two) pointing into this intersection will help make it clear that both VerifiablePresentation and verifiableCredential point to the three properties there.

We'll need to tweak the text around these diagrams a bit, and maybe elsewhere in the doc, to make it clear that these are recommendations -- e.g., that verifiableCredential could have a holder property, without making it fail verification.

@iherman
Copy link
Member Author

iherman commented Sep 4, 2023

@TallTed I hope I understood what you asked. And it is indeed better, the crossing point of the domain lines are now more visibly part of the other domain lines. Thanks.

Of course, the diagram will probably change, depending on the outcome of the open issues: the domain lines might be all common, which probably means that this crossing point will move to the "north" a bit and will have much more arrows coming out of it. Also, there may be yet another box for relatedResource.

as for

We'll need to tweak the text around these diagrams a bit, and maybe elsewhere in the doc, to make it clear that these are recommendations -- e.g., that verifiableCredential could have a holder property, without making it fail verification.

Strictly speaking, all of these are "recommendations" in RDFS, which shouldn't be used for verification or resulting in failure. That is SHACL land, except that SHACL (or ShEx) are not prepared to express the "contain" relationship.

Actually, it would require OWL (using property cardinalities) to reinforce the things you ask for, something that says that, for example, the VerifiableCredential class is a subclass of the one characterized by the fact that there is at least one credentialSubject on it. But we did not introduce OWL ontology statements on the vocabulary. Apart from the fact that my tool is not prepared to it, do we really want to go there?

@TallTed
Copy link
Member

TallTed commented Sep 5, 2023

I hope I understood what you asked

Yes, that's much better. Every little bit helps!

@iherman
Copy link
Member Author

iherman commented Sep 8, 2023

I believe that with the PRs #189 and w3c/vc-data-model#1268 the diagram changes have been pushed "back" to the main branches, and this issue could be closed.

@iherman iherman added the pending close (7 days) This issue will be closed after 7 days. label Sep 8, 2023
@brentzundel
Copy link
Member

No objections raised to closing since marked as pending close, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR1 This item was processed during the first Candidate Recommendation phase. pending close (7 days) This issue will be closed after 7 days.
Projects
None yet
Development

No branches or pull requests

5 participants