Skip to content

Chore: Migrate NotificationPreferencesView to hooks - #4327

Merged
dnlsilva merged 15 commits into
developfrom
notification-preferences-view
Aug 1, 2022
Merged

Chore: Migrate NotificationPreferencesView to hooks#4327
dnlsilva merged 15 commits into
developfrom
notification-preferences-view

Conversation

@dnlsilva

Copy link
Copy Markdown
Contributor

Proposed changes

Migrate NotificationPreferencesView to hooks and improves layout and user feedback

  • It is no longer necessary to change screens to select some options, they are via actionSheet.
  • Now when an error occurs when changing the option, it reverts to the old option.

Issue(s)

How to test or reproduce

  • Try changing some option and see if it will actually replicate on the web or in tests.
  • Force an error (too many requests for example) and see the error message and the go back option.

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

@dnlsilva
dnlsilva force-pushed the notification-preferences-view branch from db6670c to e8ae963 Compare June 28, 2022 19:46
@lgtm-com

lgtm-com Bot commented Jun 28, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging e8ae963 into e48527b - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

Comment thread app/containers/ActionSheet/Item.tsx Outdated
Comment thread app/containers/ActionSheet/styles.ts Outdated
Comment thread app/views/NotificationPreferencesView/index.tsx Outdated
Comment thread app/views/NotificationPreferencesView/index.tsx
Comment thread app/views/NotificationPreferencesView/index.tsx
Comment thread app/views/NotificationPreferencesView/index.tsx Outdated
@lgtm-com

lgtm-com Bot commented Jul 19, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 4ff9384 into 60f781d - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com

lgtm-com Bot commented Jul 22, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 3e9d1ee into 376c8e8 - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

testID={preference as string}
trackColor={SWITCH_TRACK_COLOR}
onValueChange={value => {
onChangeValue(preference, { [preference]: switchValue ? '1' : '0' }, () => setSwitchValue(switchValue));

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.

WDYT to use named params?

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.

named params?

Comment thread app/views/NotificationPreferencesView/index.tsx Outdated
@lgtm-com

lgtm-com Bot commented Jul 25, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging bcf0480 into 376c8e8 - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com

lgtm-com Bot commented Jul 27, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 92e8f5c into 376c8e8 - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com

lgtm-com Bot commented Jul 28, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging d606718 into 376c8e8 - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

@lgtm-com

lgtm-com Bot commented Aug 1, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging e56c14e into 5875454 - view on LGTM.com

fixed alerts:

  • 1 for Direct state mutation

@dnlsilva
dnlsilva merged commit 41dbbf4 into develop Aug 1, 2022
@dnlsilva
dnlsilva deleted the notification-preferences-view branch August 1, 2022 16:35
diegolmello pushed a commit that referenced this pull request Sep 12, 2022
* add none option

* create showErrorAlertWithEMessage function

* migrate NotificationPreferencesView to hooks and improves overall

* change icon to right

* fix navigation options

* remove none

* fix types

* remove memo

* cleaning

* switching to destructuring

* add observe for hideUnreadStatus

* fix desktop options

* remove unused options

Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com>
navislavo pushed a commit to NextiaDev/Rocket.Chat.ReactNative that referenced this pull request May 26, 2023
* add none option

* create showErrorAlertWithEMessage function

* migrate NotificationPreferencesView to hooks and improves overall

* change icon to right

* fix navigation options

* remove none

* fix types

* remove memo

* cleaning

* switching to destructuring

* add observe for hideUnreadStatus

* fix desktop options

* remove unused options

Co-authored-by: Reinaldo Neto <47038980+reinaldonetof@users.noreply.github.com>
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.

3 participants