w3c / vc-data-integrity

W3C Data Integrity Specification
https://w3c.github.io/vc-data-integrity/
Other
42 stars 19 forks source link

Apply diagram changes #175

Closed iherman closed 1 year ago

iherman commented 1 year ago

This is just a warning place to adopt some changes on the vocabulary diagrams, see https://github.com/w3c/vc-data-integrity/pull/171#issuecomment-1686920335 and the followups in https://github.com/w3c/vc-data-integrity/pull/171#issuecomment-1687382851 and https://github.com/w3c/vc-data-integrity/pull/171#issuecomment-1688493363

iherman commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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:

None of these are good alternatives...

TallTed commented 1 year ago

@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 commented 1 year ago

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

iherman commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

darkmode-di-diagram

iherman commented 1 year ago

@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 https://github.com/w3c/vc-data-integrity/issues/175#issuecomment-1694241178.)

TallTed commented 1 year ago
  • 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 commented 1 year ago
  • 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 commented 1 year ago

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 commented 1 year ago

For those who have not experienced the blak art of dark mode... the example of @dlongley in https://github.com/w3c/vc-data-integrity/issues/175#issuecomment-1693633879 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 commented 1 year ago

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 https://github.com/w3c/vc-data-integrity/issues/175#issuecomment-1694196379, at also ok with the default Chromium built-in flag).

It seems that I was pessimistic in https://github.com/w3c/vc-data-integrity/issues/175#issuecomment-1693713602. @dlongley, if it is also acceptable for you, then we could consider that part of the changes done. WDYT?

dlongley commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Yes.

iherman commented 1 year ago

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

(I am out for the rest of the week)

TallTed commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

I hope I understood what you asked

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

iherman commented 1 year ago

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

brentzundel commented 1 year ago

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