ER and Sequence Chart Accessibility #2832
Conversation
|
Good stuff! We really appreciate efforts towards accessibility and want to merge this ... but... there were a couple of things that needs to be adjusted for us to be able to go forward. The first one is that we need to sanitise the data coming in for the titles to be sure there are no harmfull data in there. You can look at usage of the sanitizeText function in sequenceDb. The second issue is that the syntax for the title in sequence diagrams are updated. I agree that it is more consistent without the ":" but we have existing diagrams out there, using the old syntax. These diagrams would break from this PR and annoying as it is we take backwards compatibility in diagrams seriously. This what I am referring to: and this
So if you can fix those things I will be happy to proceed. |
| @@ -56,7 +56,9 @@ | |||
| "note" return 'note'; | |||
| "activate" { this.begin('ID'); return 'activate'; } | |||
| "deactivate" { this.begin('ID'); return 'deactivate'; } | |||
| "title" return 'title'; | |||
| "title"\s[^#\n;]+ return 'title'; | |||
| "title:"\s[^#\n;]+ return 'legacy_title'; | |||
There was a problem hiding this comment.
@knsv I added support for the new and the old style title. I think that it would be good to have a consistent dsl. Is this approach ok?
| @@ -122,9 +122,6 @@ export const getActorKeys = function () { | |||
| export const getTitle = function () { | |||
| return title; | |||
| }; | |||
| export const getTitleWrapped = function () { | |||
There was a problem hiding this comment.
I do not think we need this title wrapped logic any more?
馃搼 Summary
This adds titles to ER documents and Sequence documents. There is a weird thing with the sequenceDiagram.spec.js where the element passed in as the diagram seems to be a stub? So I just ensure that the element passed to the accessibility function will respond to insert.
馃搹 Design Decisions
Following the already set pattern for adding accessibility to charts, setup in #2732
馃搵 Tasks
Make sure you
developbranch