fedidcg / FedCM

A privacy preserving identity exchange Web API
https://fedidcg.github.io/FedCM
Other
357 stars 66 forks source link

ASCII art? #332

Open martinthomson opened 1 year ago

martinthomson commented 1 year ago

The diagrams in the current version of the document appear to have been generated with some ASCII art tool. This would appear to make copy and paste easier, but it does not. If ASCII versions are important, consider using a tool like aasvg (shameless plug) to generate SVG from the ASCII art.

samuelgoto commented 1 year ago

This would appear to make copy and paste easier, but it does not If ASCII versions are important

It is not copying and pasting that needs to be made easier, but collaborative editing.

The diagrams currently exist in a google docs that we use to edit and export to SVG. Example:

https://fedidcg.github.io/FedCM/#other-attack-scenarios https://fedidcg.github.io/FedCM/#secondary-use

Needless to say, that isn't scaling very well as we add more edits into the spec (e.g. you can't ask "hey, update the diagrams too" in code reviews).

So, we thought that using ASCII and github would allow anyone to edit them collaborative in things like PRs.

I'm slowly porting them one by one, although we got some of the most important / highly mutable ones.

consider using a tool like aasvg (shameless plug) to generate SVG from the ASCII art.

Will do.

I'm looking for a few options and alternatives, but I'm convinced that we should find a way to allow for the diagrams to be collaboratively editable through code reviews.

martinthomson commented 1 year ago

It is not copying and pasting that needs to be made easier, but collaborative editing.

Priority of constituencies might say otherwise. FWIW, the accessibility of the diagrams is terrible, and that is for me. Worse, there are tables in the documented that are produced with this tool. Tables are a solved problem.

samuelgoto commented 1 year ago

Priority of constituencies might say otherwise. FWIW, the accessibility of the diagrams is terrible, and that is for me.

Honestly, we don't feel strongly what tool we use for diagrams. We could export them to SVGs (that's what they were originally, but they were hard to main), but we wouldn't be increasing accessibility.

Would you prefer that we use / exported as SVGs instead?

Worse, there are tables in the documented that are produced with this tool. Tables are a solved problem.

Agreed that tables shouldn't be diagrams, sending a PR to convert the tables.

martinthomson commented 1 year ago

I'm just looking for better a11y for those figures, that's all. If you have ASCII art source and don't want to change much, then other tools can produce better output.

I tend to think that these AA->SVG translations are nice in terms of enforcing consistent presentation, but somewhat painful to edit in source. That's opinion though. I don't want to dictate the editing process, except to the extent that it has negative externalities (like really bad copy-paste).

npm1 commented 1 year ago

Related to this, @jyasskin mentioned we could use https://mermaid.js.org/. Apparently it has support within GitHub, but probably not supported in bikeshed, so we'd need to move the images to a separate file and just use the generated value in the bikeshed document.

jyasskin commented 1 year ago

aasvg is really cool, but I'm pretty sure the source for these diagrams should be semantically a sequence diagram, rather than ascii art. Either Mermaid or PlantUML would get us there, but I'd lean toward Mermaid because it's what Github picked, so we avoid adding one to the set of languages people have to learn. Tab's amenable to adding support directly to Bikeshed, once someone gets around to it: https://github.com/speced/bikeshed/issues/2539.

Davilink commented 9 months ago

Is it normal that we cannot see the diagram when using dark theme ? image

When a select the diagram with my mouse i can see there some text but not really helpful image

martinthomson commented 9 months ago

This is a consequence of Sam's tooling being a little less mature than some alternatives. See https://github.com/blampe/goat/issues/23 or maybe even https://datatracker.ietf.org/doc/html/rfc9113#StreamStatesFigure for examples of how to do this sort of thing better.

npm1 commented 6 months ago

The dark mode issue appears to be fixed by https://github.com/fedidcg/FedCM/pull/528