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: account and token header tsx and hooks #667

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

ckniffen
Copy link
Collaborator

High Level Overview of Change

  • Make <AccountHeader> and <TokenHeader> use typescript and hooks
  • Remove incorrect types
  • Remove last usages of language property on redux state
  • Fix EmailHash not showing up (types were incorrect)

Fixes #476 and related to #554

Context of Change

With the new language picker all updates to language happen directly through i18n-next. All references to language now come through the useLanguage hook instead of redux.

In order to use the hook <TokenHeader> had to be refactored to a functional component. <AccountHeader> is a very similar component (<TokenHeader> was based on it) so it make sense to complete the Typescript refactor of that component.

While setting up the component's types I fixed #476 since I did not want incorrect types. Additionally <TokenHeader> had many props defined that were copied from <AccountHeader> but are never returned by getToken.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor (non-breaking change that only restructures code)

TypeScript/Hooks Update

  • Updated files to React Hooks
  • Updated files to TypeScript

- Make `<AccountHeader>` and `<TokenHeader>` use typescript and hooks
- Remove incorrect types
- Fix EmailHash not showing up (types were bad)
- Remove last usages of language property on redux state

Fixes #476 and related to #554
paychannels: {
// eslint-disable-next-line camelcase
total_available: string
channels: any[]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous definition was PropTypes.shape({ length: PropTypes.number }). No other properties are referenced so I figured any[] array matches that. This might be something to address when following up on #587.

export default connect(
(state) => ({
language: state.app.language,
(state: any) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is just a mapping and will soon go away when redux is fully removed. The account and token pages are the last ones using redux for data. The other redux usages will be removed with the new header.

}

export default connect(
(state) => ({
language: state.app.language,
(state: any) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same answer as <AccountHeader>.

@ckniffen ckniffen merged commit aae9663 into staging Apr 13, 2023
@ckniffen ckniffen deleted the refactor/remove-language-from-redux branch April 13, 2023 00:37
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.

Email hash may not be ever displayed in account header
3 participants