Skip to content

Issues & suggestions #6

@PolinaGo

Description

@PolinaGo

Hi! Thanks for the codelab! It was interesting and useful. Below are some issues & suggestions:

1. Setup

  1. Issue: While completing this codelab on a MacBook, I ran into an issue following the setup steps.
    The issue: npm install (step 5) fails due to conflicting peer dependencies. Running npm install --force works. It seems like either dependencies or instructions should be updated.
  2. Improvement: It would be useful to additionally specify (before or during step 5) that installing npm is a prerequisite for running npm install.

5. Ensure adequate color contrast
Conceptual: Although we fixed the contrast for the home/view type icons, we haven't solved all the high contrast issues on the page i.e. the number picker. It's strange to mark it as 'Done'

7. Create selectable controls with Angular Material
Conceptual: although we have solved the a11y concern, we had to change the intended content of the page. It would be great to show how we can use a11y to improve experience without changing what we need to convey.

10. Control focus with FocusTrap
Issue: For me, the focus is already trapped and does not exit the dialog. Adding cdkFocusInitial did not change anything.
Also, Focus is now initially set on Change Color in the dialog! doesn't make sense because there is no element Change Color.

11. Announce changes with LiveAnnouncer
Improvement: In src/app/shop/color-picker/color-picker-dialog/color-picker-dialog.component.ts, Inject and MAT_DIALOG_DATA should be additionally imported (this might be worth putting into the snippet).

12. Enable HighContrast mode
Issue: Copying code directly produces SassError: Undefined mixin. for cdk-high-contrast(). As per Angular Material docs it should be cdk.high-contrast(), and then the page works.

Additional suggestions:

  1. Improvement: It would be useful to separate ungrouped line changes into different snippets (even within the same file).
  2. Improvement: // TODO: #5. Ensure adequate color contrast in the codebase should have a tailing period to align with the codelab (to search for TODO in the codebase). Same goes for the following TODOs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions