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

Feature/wd3691 #36

Merged
merged 3 commits into from Jan 12, 2022
Merged

Feature/wd3691 #36

merged 3 commits into from Jan 12, 2022

Conversation

Baelx
Copy link
Contributor

@Baelx Baelx commented Jan 12, 2022

image

This is a partial PR. What is to come for this feature branch:

  • Add more tests for Users, RowEditDeleteChip, etc. (currently blocked by Material UI causing issues with Jest)
  • Improve keyboard and screen reader experience for Users table.

This PR is a little on the large side. Basically this comes down to:

Caveats:

  • Much of the data that should be pulled from the DB is hard-coded for now.
  • There seems to be an issue using Sass's @use. I'm using scss imports for now.

// import red from '@mui/material/colors/red';

// Callbacks can have any number of arguments and can have any return type.
interface IProps {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though we weren't using Typescript for this app?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using it for the frontend only. I think the discussion around TS was for the backend

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure it was the whole app. we havent had a chance to all come together to hash this out yet. maybe we need a developer coffee before the next scheduled one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShawnTurple is this true?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using ts in the frontend only

frontend/src/pages/Users/index.tsx Show resolved Hide resolved
@ASpiteri-BCGov ASpiteri-BCGov merged commit 96f399a into development Jan 12, 2022
@ShawnTurple ShawnTurple deleted the feature/wd3691 branch January 12, 2022 20:12
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

4 participants