Skip to content

refactor editorAccessibility reducers and actions using redux toolkit #3120

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

Conversation

PiyushChandra17
Copy link
Contributor

Associated issue: #2042

Changes:

  • First things first removed all the associated constants
  • Used createSlice to rewrite the editorAccessibility reducer
  • Exported the automatically generated action creator functions from actions/editorAccessibility.js
  • Deleted the existing action creator function

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
@raclim
Copy link
Collaborator

raclim commented May 31, 2024

Thanks for your work on refactoring the reducers and actions to use Redux Toolkit!

I think a few of these PRs aren't passing the tests—would you be able to update those PRs to ensure that they pass them? Thanks!

@PiyushChandra17
Copy link
Contributor Author

Thanks for your work on refactoring the reducers and actions to use Redux Toolkit!

I think a few of these PRs aren't passing the tests—would you be able to update those PRs to ensure that they pass them? Thanks!

@raclim I'm quite aware that these few PR's aren't passing the tests as it should be, i'll be soon working on it and make sure that they pass the test going forward. Probably I'll update you soon!

Verified

This commit was signed with the committer’s verified signature.
message: action.message,
id: messageId
})
state.lintMessages.push({
Copy link
Collaborator

Choose a reason for hiding this comment

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

You got the mutations right here 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lindapaiste Thanks for the feedback, these are really helpful :)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…Toolkit

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…Toolkit
@raclim raclim merged commit 5226e25 into processing:develop Aug 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants