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

refactor accessibility #4551

Merged
merged 5 commits into from
Jul 3, 2023

Conversation

Yokozuna59
Copy link
Member

@Yokozuna59 Yokozuna59 commented Jun 27, 2023

📑 Summary

  • remove @ts-ignore comments
  • rename vitest test and describe title
  • remove unnecessary types, e.i, null and number
  • clean addSVGa11yTitleDescription and setA11yDiagramInfo functions

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 📓 have added documentation (if appropriate)
  • 🔖 targeted develop branch

@Yokozuna59
Copy link
Member Author

Yokozuna59 commented Jun 27, 2023

Should I update related stuff in mermaidAPI.ts and mermaidAPI.spec.ts?

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2023

Codecov Report

Merging #4551 (406df57) into develop (9c011ab) will increase coverage by 5.27%.
The diff coverage is 78.37%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4551      +/-   ##
===========================================
+ Coverage    75.10%   80.37%   +5.27%     
===========================================
  Files          135      144       +9     
  Lines        16498    16939     +441     
  Branches       510      521      +11     
===========================================
+ Hits         12390    13614    +1224     
+ Misses        3997     3220     -777     
+ Partials       111      105       -6     
Flag Coverage Δ
e2e 83.99% <100.00%> (+8.43%) ⬆️
unit 60.15% <35.13%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/mermaid/src/mermaidAPI.ts 62.20% <73.33%> (+1.09%) ⬆️
packages/mermaid/src/accessibility.ts 78.46% <81.81%> (+1.31%) ⬆️

... and 42 files with indirect coverage changes

@Yokozuna59
Copy link
Member Author

I'm confused, aren't these identical?

describe('render', () => {
// These are more like integration tests right now because nothing is mocked.
// But it is faster that a cypress test and there's no real reason to actually evaluate an image pixel by pixel.
// render(id, text, cb?, svgContainingElement?)
// Test all diagram types. Note that old flowchart 'graph' type will invoke the flowRenderer-v2. (See the flowchart v2 detector.)
// We have to have both the specific textDiagramType and the expected type name because the expected type may be slightly different than was is put in the diagram text (ex: in -v2 diagrams)
const diagramTypesAndExpectations = [
{ textDiagramType: 'C4Context', expectedType: 'c4' },
{ textDiagramType: 'classDiagram', expectedType: 'classDiagram' },
{ textDiagramType: 'classDiagram-v2', expectedType: 'classDiagram' },
{ textDiagramType: 'erDiagram', expectedType: 'er' },
{ textDiagramType: 'graph', expectedType: 'flowchart-v2' },
{ textDiagramType: 'flowchart', expectedType: 'flowchart-v2' },
{ textDiagramType: 'gitGraph', expectedType: 'gitGraph' },
{ textDiagramType: 'gantt', expectedType: 'gantt' },
{ textDiagramType: 'journey', expectedType: 'journey' },
{ textDiagramType: 'pie', expectedType: 'pie' },
{ textDiagramType: 'requirementDiagram', expectedType: 'requirement' },
{ textDiagramType: 'sequenceDiagram', expectedType: 'sequence' },
{ textDiagramType: 'stateDiagram-v2', expectedType: 'stateDiagram' },
];
describe('accessibility', () => {
const id = 'mermaid-fauxId';
const a11yTitle = 'a11y title';
const a11yDescr = 'a11y description';
diagramTypesAndExpectations.forEach((testedDiagram) => {
describe(`${testedDiagram.textDiagramType}`, () => {
const diagramType = testedDiagram.textDiagramType;
const diagramText = `${diagramType}\n accTitle: ${a11yTitle}\n accDescr: ${a11yDescr}\n`;
const expectedDiagramType = testedDiagram.expectedType;
it('aria-roledscription is set to the diagram type, addSVGa11yTitleDescription is called', async () => {
const a11yDiagramInfo_spy = vi.spyOn(accessibility, 'setA11yDiagramInfo');
const a11yTitleDesc_spy = vi.spyOn(accessibility, 'addSVGa11yTitleDescription');
await mermaidAPI.render(id, diagramText);
expect(a11yDiagramInfo_spy).toHaveBeenCalledWith(
expect.anything(),
expectedDiagramType
);
expect(a11yTitleDesc_spy).toHaveBeenCalled();
});
});
});
});
});

And

describe('render', () => {
// Be sure to add async before each test (anonymous) method
// These are more like integration tests right now because nothing is mocked.
// But it is faster that a cypress test and there's no real reason to actually evaluate an image pixel by pixel.
// render(id, text, cb?, svgContainingElement?)
// Test all diagram types. Note that old flowchart 'graph' type will invoke the flowRenderer-v2. (See the flowchart v2 detector.)
// We have to have both the specific textDiagramType and the expected type name because the expected type may be slightly different than was is put in the diagram text (ex: in -v2 diagrams)
const diagramTypesAndExpectations = [
{ textDiagramType: 'C4Context', expectedType: 'c4' },
{ textDiagramType: 'classDiagram', expectedType: 'classDiagram' },
{ textDiagramType: 'classDiagram-v2', expectedType: 'classDiagram' },
{ textDiagramType: 'erDiagram', expectedType: 'er' },
{ textDiagramType: 'graph', expectedType: 'flowchart-v2' },
{ textDiagramType: 'flowchart', expectedType: 'flowchart-v2' },
{ textDiagramType: 'gitGraph', expectedType: 'gitGraph' },
{ textDiagramType: 'gantt', expectedType: 'gantt' },
{ textDiagramType: 'journey', expectedType: 'journey' },
{ textDiagramType: 'pie', expectedType: 'pie' },
{ textDiagramType: 'requirementDiagram', expectedType: 'requirement' },
{ textDiagramType: 'sequenceDiagram', expectedType: 'sequence' },
{ textDiagramType: 'stateDiagram-v2', expectedType: 'stateDiagram' },
];
describe('accessibility', () => {
const id = 'mermaid-fauxId';
const a11yTitle = 'a11y title';
const a11yDescr = 'a11y description';
diagramTypesAndExpectations.forEach((testedDiagram) => {
describe(`${testedDiagram.textDiagramType}`, () => {
const diagramType = testedDiagram.textDiagramType;
const diagramText = `${diagramType}\n accTitle: ${a11yTitle}\n accDescr: ${a11yDescr}\n`;
const expectedDiagramType = testedDiagram.expectedType;
it('aria-roledscription is set to the diagram type, addSVGa11yTitleDescription is called', async () => {
const a11yDiagramInfo_spy = vi.spyOn(accessibility, 'setA11yDiagramInfo');
const a11yTitleDesc_spy = vi.spyOn(accessibility, 'addSVGa11yTitleDescription');
await mermaidAPI.render(id, diagramText);
expect(a11yDiagramInfo_spy).toHaveBeenCalledWith(
expect.anything(),
expectedDiagramType
);
expect(a11yTitleDesc_spy).toHaveBeenCalled();
});
});
});
});
});


The only difference is this comment:

// Be sure to add async before each test (anonymous) method

online differ: https://linediff.com/?id=649b1093687f4b8d478b4567

- remove @ts-ignore comments
- rename vitest test and describe title
- remove unnecessary types, e.i, `null` and `number`
- clean `addSVGa11yTitleDescription` and `setA11yDiagramInfo` functions
@sidharthv96
Copy link
Member

@Yokozuna59 yes, those are identical. Maybe, I copied them when render and renderAsync was present and we didn't remove them afterwards.

@sidharthv96 sidharthv96 added this pull request to the merge queue Jul 3, 2023
Merged via the queue into mermaid-js:develop with commit 1b190b2 Jul 3, 2023
15 checks passed
@Yokozuna59 Yokozuna59 deleted the refactor-accessibility branch July 3, 2023 15:39
github-merge-queue bot pushed a commit to fuxingloh/contented that referenced this pull request Jul 31, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.2.4` ->
`10.3.0`](https://renovatebot.com/diffs/npm/mermaid/10.2.4/10.3.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.2.4/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.2.4/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mermaid-js/mermaid (mermaid)</summary>

###
[`v10.3.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.3.0):
10.3.0

[Compare
Source](https://togithub.com/mermaid-js/mermaid/compare/v10.2.4...v10.3.0)

#### What's Changed

##### Features

- Sankey diagrams by [@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4502
- Feature/1838 actor creation destruction by
[@&#8203;Valentine14th](https://togithub.com/Valentine14th) in
[mermaid-js/mermaid#4466
- Vertical branches in Git Diagram by
[@&#8203;mastersibin](https://togithub.com/mastersibin) in
[mermaid-js/mermaid#4639
- Use JSON Schema to define and document `MermaidConfig` by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4112
- Remove the test checking whether the JSON Schema default config
matched the old default config by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4610
- Fixes support of the macro `ContainerQueue_Ext` for C4 diagrams
definition. by [@&#8203;kislerdm](https://togithub.com/kislerdm) in
[mermaid-js/mermaid#4577

##### Bugfixes

- Make quadrant chart options TypeScript types optional by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4602
- Remove double parsing by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4587
- Fix flowchart tooltip typing bug by
[@&#8203;lishid](https://togithub.com/lishid) in
[mermaid-js/mermaid#4562
- Bug/4590 allow notes identical to keywords by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4597
- feat: allow specifying on which weekday a tickInterval should start by
[@&#8203;leinelissen](https://togithub.com/leinelissen) in
[mermaid-js/mermaid#4634
- Split formatted markdown strings with unicode support. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4470
- fix: Mind maps handles `-` signs in node ids/text by
[@&#8203;knsv](https://togithub.com/knsv)

##### Chores

- Remove all TypeScript enums and forbid them in ESLint by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4580
- refactor accessibility by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#4551
- chore: Reduce codecov pushes by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4604
- Run PR-labeler-config-validator only if config changes by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4607
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4624
- Update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4566
- Update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4581
- Rename workflow jobs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4574
- Removed unused code in state diagrams by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4631
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4623
- chore: remove unused `devDependency` on coveralls by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4641
- Allow entity diagram attribute names to start with asterisk by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4588
- Bug/4592 fix new line padding class diagram by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4633
- Fix graph not loading when the img loads too fast or fail to load by
[@&#8203;pierrickouw](https://togithub.com/pierrickouw) in
[mermaid-js/mermaid#4496
- convert `cypress/helpers/util.js` to ts by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#4552
- build(deps-dev): bump word-wrap from 1.2.3 to 1.2.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[mermaid-js/mermaid#4652
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4663
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4662

##### Documentation

- Sankey: Remove duplicated examples by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4595
- Release docs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4493
- Update latest news section by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4495
- Fix Typo by [@&#8203;ryru](https://togithub.com/ryru) in
[mermaid-js/mermaid#4567
- Docs: add ChatGPT plugin blog post by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4570
- Fix relative link to theme variables list by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4573
- Fix docs:dev by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4598
- Docs: update link - "Join the Community" by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4601
- Support docs:dev in docker by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4599
- docs(flowchart): add documentation on multiple nodes style by
[@&#8203;tomperr](https://togithub.com/tomperr) in
[mermaid-js/mermaid#4600
- Avoid downloading avtars everytime on docs:dev by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4603
- docs: Fix checkbox syntax by
[@&#8203;guilhermgonzaga](https://togithub.com/guilhermgonzaga) in
[mermaid-js/mermaid#4646
- Fix the "Edit this page on GitHub" link in Vitepress documentation for
the Mermaid Config pages by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4640
- Support MERMAID_RELEASE_VERSION in docs. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4612
- Docs: update Latest News section by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4655
- added Typora to integrations list by
[@&#8203;kgilbert78](https://togithub.com/kgilbert78) in
[mermaid-js/mermaid#4666
- Docs: Corrects name of C4 link by
[@&#8203;Incognito](https://togithub.com/Incognito) in
[mermaid-js/mermaid#4660
- Fix a typo by [@&#8203;gjtorikian](https://togithub.com/gjtorikian) in
[mermaid-js/mermaid#4396

#### New Contributors

- [@&#8203;ryru](https://togithub.com/ryru) made their first
contribution in
[mermaid-js/mermaid#4567
- [@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) made
their first contribution in
[mermaid-js/mermaid#4573
- [@&#8203;kislerdm](https://togithub.com/kislerdm) made their first
contribution in
[mermaid-js/mermaid#4577
- [@&#8203;leinelissen](https://togithub.com/leinelissen) made their
first contribution in
[mermaid-js/mermaid#4634
- [@&#8203;pierrickouw](https://togithub.com/pierrickouw) made their
first contribution in
[mermaid-js/mermaid#4496
- [@&#8203;mastersibin](https://togithub.com/mastersibin) made their
first contribution in
[mermaid-js/mermaid#4639
- [@&#8203;kgilbert78](https://togithub.com/kgilbert78) made their first
contribution in
[mermaid-js/mermaid#4666
- [@&#8203;Incognito](https://togithub.com/Incognito) made their first
contribution in
[mermaid-js/mermaid#4660
- [@&#8203;gjtorikian](https://togithub.com/gjtorikian) made their first
contribution in
[mermaid-js/mermaid#4396

**Full Changelog**:
mermaid-js/mermaid@v10.2.4...v10.3.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/contented).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants