Skip to content

Chore: Migrate App/index.tsx to hooks - #4401

Closed
reinaldonetof wants to merge 11 commits into
developfrom
chore.hooks-app-index
Closed

Chore: Migrate App/index.tsx to hooks#4401
reinaldonetof wants to merge 11 commits into
developfrom
chore.hooks-app-index

Conversation

@reinaldonetof

@reinaldonetof reinaldonetof commented Jul 30, 2022

Copy link
Copy Markdown
Contributor

Proposed changes

Migrate the App/index.tsx to hooks.

This is the second file, after the index.js. Here we start with a lot of providers, the redux and some modals.

Issue(s)

How to test or reproduce

  • Test the themes, change them to another, and back
  • Test the deeplink
  • Test when dimension changes
  • Test the tablet
  • Test the theme in share extension

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Jira: NATIVE-45

@guilhermedfs guilhermedfs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@liversonp

Copy link
Copy Markdown

Hello. I was reviewing about hooks in react native and I found this way to hook consts using usereducer, I think this can help in clean code

Comment thread app/index.tsx
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/share.tsx Outdated
// change theme state
this.setState(
prevState => newThemeState(prevState, newTheme as IThemePreference),
prevState => newThemeState(prevState.themePreferences, newTheme as IThemePreference),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/RocketChat/Rocket.Chat.ReactNative/pull/4401/files#diff-b788f0c6b6e92a2a43dedd61dd4e596b4b0f981e30bcfd88e2f5d30b2de3a7dbR39

Because I changed the function newThemeState to receive only the property required, not the state completely.

Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/index.tsx Outdated
Comment thread app/lib/methods/helpers/theme.ts Outdated

@dnlsilva dnlsilva left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add a comment to your task saying that it has to be well tested.
All app works.
Open and close, push etc.

@diegolmello
diegolmello deleted the chore.hooks-app-index branch March 11, 2025 18:59
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.

5 participants