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

Improper ref use warnings on LoginScreen and general refactoring #704

Closed
MaxwellDG opened this issue Jul 19, 2022 · 8 comments
Closed

Improper ref use warnings on LoginScreen and general refactoring #704

MaxwellDG opened this issue Jul 19, 2022 · 8 comments

Comments

@MaxwellDG
Copy link
Contributor

Hi everyone,

I'm new to this repo and was sent here by @mikeallbutt. I'm part of the Puerto Escondido chapter down here and was excited to hear that you have an app in development. I'm a professional frontend developer with 5 years experience (2 years in React Native) and I'd like to contribute where I can.

To get more familiar with the general feel and workflow of the app, I decided to look at LoginScreen first. Especially because there were some warnings there on startup.

Issue:

  • Warnings on app loading about improper ref usage
  • General refactoring for code bloat on LoginScreen.js
  • Several components could be combined into one which would have a larger amount of reusability across the app (Namely 'LabeledTextInput.js')
@zdmc23
Copy link
Collaborator

zdmc23 commented Jul 19, 2022

Hi @MaxwellDG , we are happy to have your help! Thanks for jumping right in and reviewing the code!

It's up to you whether you want to refactor LoginScreen just yet, bc we have a change coming soon to punt login to the Web via OAuth2 (we have a PoC - the open web browser flow, and then redirect control back to the app once authenticated). The reason for this is that we have received several requests for 2FA/MFA and other authentication-based plugins, and it's not feasible for us to support them all in-app. We did have an initial 2FA implementation, and it works fine, but we needed to standardize on a particular 2FA WordPress plugin in order to validate the code according to that plugin's DB table name, schema, etc... It quickly becomes impractical when we're supporting hundreds of instances all using a different set of auth plugins that we cannot control. We could modify our Mobile App plugin to be OAuth2 Server compliant, but haven't had the time, and also some teams want to stick with what they already have installed. TL;DR: it is simpler to just punt authentication to the Web and then teams can have any combination of plugins they want, or use Azure AD (without us needing to figure out how to get the appropriate Tenant IDs, etc...)

All that to say that Login may be changing significantly, so I leave it up to you how much time you want to invest there. Maybe there are some quick wins around ref forwarding, and component reuse and if so, go for it. Thx!

@MaxwellDG
Copy link
Contributor Author

MaxwellDG commented Jul 19, 2022

That's good to know. I should probably ask beforehand next time. I've written something up right now and I'll submit it tomorrow night. It sounds like it'll get overwritten soon but it's cool. It was mostly for me to have a specified task while I browsed the codebase.

PS It's funny cause I'm actually doing the opposite at work right now. I'm removing a webview 0Auth login lol. Boss wants very specific styling, and 0Auth webview login doesn't allow for any customization there. It's certainly very effective auth though.

@zdmc23
Copy link
Collaborator

zdmc23 commented Jul 20, 2022

No worries, I realize we currently have very little documentation to help onboard to the project. Once we get this release out, I will try to get on that and highlight some rationale and maybe where a decision seemed good at first but could be improved upon, etc...

PS. We still may end up keeping the Basic Auth flow for teams who do not want to deal with an extra OAuth2 Provider plugin, so I don't think any of your work will be in vain.

PSS. Check out https://github.com/DiscipleTools/disciple-tools-mobile-app/blob/development/components/Field/Text/TextField.js , there may also be some opportunity to reuse any text input components there too

@MaxwellDG
Copy link
Contributor Author

@zdmc23 Hey is there another place we could discuss the project? Are you using Slack, or Trello, or Asana or anything like that? A discord channel maybe?

Cause I've been trying to do 2 smaller issues (#700 and #688) and as far as I can tell they've already been completed. I'd rather not keep searching through the issues if lots of them are already completed.

@zdmc23
Copy link
Collaborator

zdmc23 commented Jul 22, 2022

@MaxwellDG , we have a private Slack, please DM me an email that you want me to use for invite. Thx

@MaxwellDG
Copy link
Contributor Author

MaxwellDG commented Jul 22, 2022

@zdmc23 maxwellmdg@protonmail.com. Does GitHub even have a direct messaging feature? Lol I can't find it.

@MaxwellDG MaxwellDG mentioned this issue Jul 22, 2022
@zdmc23 zdmc23 closed this as completed in 71b1401 Jul 22, 2022
@zdmc23
Copy link
Collaborator

zdmc23 commented Jul 22, 2022

@MaxwellDG Thx for the PR on this, it looks better! Just one thing, when I change the language, the TextField labels are not being updated bc of the memoization being based on the key (which doesn't change - same English key regardless of locale):

const text = React.useMemo(() => i18n.t(i18nKey), [i18nKey])

If we still want to memoize, then something like the following would prob do the trick:

const text = React.useMemo(() => i18n.t(i18nKey), [i18n?.locale])

Thx!

@zdmc23 zdmc23 reopened this Jul 22, 2022
zdmc23 added a commit that referenced this issue Jul 27, 2022
* reference app name as a constant

* chevron icon w/ RTL support

* enable swipe, wrap-around swipe, and lift-up state to prevent re-render of index

* adjust badge offset for Android

* update "Name" icon to be more general and represent various post types (per MexSurfer)

* improve navigation and fix navigation bugs

* Icons and prettier (#620)

* Only Prettier

* Prettier

* Icon for coaches field

* add padding to offline bar

* Setup offlineBar

* New icon for name field

* Revert "New icon for name field"

This reverts commit 40a7db7.

* Corrected icon for training

* Icons: training & parent & name

* meeting time logo in training post type

* I18n.t setup for languages  (#621)

* Only Prettier

* Prettier

* Icon for coaches field

* add padding to offline bar

* Setup offlineBar

* New icon for name field

* Revert "New icon for name field"

This reverts commit 40a7db7.

* Corrected icon for training

* Icons: training & parent & name

* meeting time logo in training post type

* translate Components with i18n

* translate Comments and Activity Component with i18n

* Prettier & translate hooks with i18n

* Fix after bad push

Co-authored-by: Zac Mac <191707+zdmc23@users.noreply.github.com>

* 5 icons for the Four Fields tab in a group

* addNewContact icon

* support API updates or Notifications

* better scrolling for Android, remove Sheet Header in scroll view

* offline filter support

* add default options for Kebab

* translation updatess

* named Android detection

* style updates and add gutters where appropriate

* ensure Home tab button click re-renders Home (rather than prev screen in Home Stack)

* reintroduce Comments and Activity support

* enable contentContainerStyle for Tiles

* update deprecated property

* resolve Comments bug with missing ID value

* controls styling and display

* ConnectionField bugfixes

* reintroduce TDM design for GroupView support

* FieldTypes.LOCATION: map-marker (#623)

* Add UserIcon to be used on the login screen

* fix GroupView scroll bug

* reimplement church health viz (view only)

* (temporarily?) reuse multi-select component to edit church health

* remember user filter selections (across every post type)

* consistently refer to name-conflict local fns with underscore

* move notifications back to tab bar

* fix "forgot password" link bug; new Button and Link components

* change AccountIcon to UserIcon in Login screen

* support add new for all post types

* do not display chip when no user is assigned

* Colors for status fields in lists (#624)

* New colors for status dots in list view

* Dynamic colors for status's in lists

* support determining whether is device or simulator

* indicator highlight for bottom tabs

* revert back to previous tab component for more features

* Bump plist from 3.0.4 to 3.0.5 (#625)

Bumps [plist](https://github.com/TooTallNate/node-plist) from 3.0.4 to 3.0.5.
- [Release notes](https://github.com/TooTallNate/node-plist/releases)
- [Changelog](https://github.com/TooTallNate/plist.js/blob/master/History.md)
- [Commits](https://github.com/TooTallNate/node-plist/commits)

---
updated-dependencies:
- dependency-name: plist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix list display bug for users and locations fields

* preliminary support for any custom post type

* center etcetera indicator

* fix bug caused by circular dependencies in use-type and use-custom-post-types

* remove RefreshControl on Home to make less jittery

* remove lazy render option on tab scroll bc it makes text fields unusable on Android

* resolve keyboard auto dismiss issue - see: satya164/react-native-tab-view#1181

* Theme and style changes (#626)

* better handle custom post types per: DiscipleTools/disciple-tools-theme#1617

* external link icon for kebabs

* reintroduce support for import contacts from phone

* update required banner

* enable favorite toggle from details screen

* fix sort

* theme the alert icon

* New icons

* New icons & adjusted cancel, close

* done, cancel, close (accept, decline)

* Use CommentActivityIcon (from CommentEditIcon)

* debug comments losing status on expand

* fix persistent filter bug with comments<->contacts

* fix bug with toggling theme

* Corrected links to help documents + formatted icons file (#628)

* Group icons - black and white (from DT dark blue)

parent, peer, child, type & types

* Reverted cancel icon to be a X

The icon named cancel is not common so it is best to use the X to indicate cancel.

* removed extra ; from color code in 2 places

* update language files

* update translation terms, align LockIcon, and remove print stmts

* no need to force locale in this component

* remove registration of push notifications from App to navigation/AppNavigator where we have access to "navigation" object to navigate app to applicable post record

* improved i18n

* Create people-groups.svg

* Create baptise.svg

* reintroduce push notification support

* i18n for date strings, add FAB to comments/activities screen

* auto-size bottom sheets

* introduce haptic feedback for certain buttons (eg, favorites), meatball w/ sheet for post items

* bugfixes

* Bump async from 2.6.3 to 2.6.4 (#629)

Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4.
- [Release notes](https://github.com/caolan/async/releases)
- [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md)
- [Commits](caolan/async@v2.6.3...v2.6.4)

---
updated-dependencies:
- dependency-name: async
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* upgrade to Expo v44

* comply with RN render errors

* more responsive notification updates, remove save toast for more native feel

* performance improvements (memoization, pure components, etc...)

* setup icons for user tab + fab

* svg icons for groups

* bugfixes and style changes

* introduce UserScreen (app settings/prefs, personal notes, more D.T settings to come

* performance improvements

* reintroduce more responsive PendingContactCard

* include icons for fixed filters (Notifications, Comments/Activities) and bottom sheets (Item meatball)

* reintroduce cancel button on bottom sheets

* ref: 6276c6d

* support for locally packaged svg icons

* peer, child icons svg

* update child.svg

* Update of people-groups.svg

simple re-export to remove width and heights (like the other svgs have)

* Better icon for the trees icon

* new svg (people groups, group type)

* Different members icon

* updated communication icons (address, other and else)

* removed unneeded iconStyle = { transform: [{ scaleX: -1 }] }

* Removal of unused png files

* removal of references to unused png icons

* fix bug with onDone() in sheet footer

* renamed icon to baptismsIcon

* fix typo

* added svg TypeIcon for certain fields

* list performance improvements

* memoizing on fields was preventing MoreScreen from rendering custom post type buttons immediately

* fix broken filter switch, improve filter default selection, styles, check string representation in useEffect dep array

* Title for `My User` screen

* Read and Unread icons for Notifications

* Rename baptism icons, reorder of list to match field order in app

* Typo: baptizeIcon (not baptiseIcon)

* memoize by value so as to not recv an error about stringifying "cyclic structures"

* Added ActivityIcon

* Icons: baptism_date, archive

* only display update req alert on status tabview

* vibrate only on save button press (\rather than state updates in some cases)

* rename PostLink to PostChip

* safety check for defaultIndex

* fix bug for missing contact record options in details sheets

* fix border edges on KeySelect field

* add vibration feedback on Chip remove

* remove redundant view (pressable inherits view)

* fix custom post type bug

* added More screen kebab menu

* View on web home screen URL mod.

This will still take the user to the dashboard (if that plugin is installed) which is like the app home screen, or to whatever DT uses as the root.

* Kebab on create/new record screen

* add Contact type icons

* Standardised the kebab menu

It was the only screen using a different way of appearing which caused it to be in a slightly different position. Now it is the same as all others.

* Kebab for import contact screen

* include created and modified dates on list items, i18n number formatting, improved date handling, and styles

* leave post header visible when opening bottom sheets (for record context)

* truncate title

* improve ux when loading bottom sheet with "Done" option

* bugfix per AssignedTo field

* bugfix for removing AssignedTo chip, and cleanup

* documentation links in kebab to Create, Details, List screens

* bugfix to handle multi-select field values onDone

* get screen width dynamically

* see: 017d76c

* bugfix for undefined status and introduce info icons in list items

* bugfix for user select chip links

* reorg info icons as subtitle in lists

* bugfix for PostChip links, simplyify ConnectionField, enable Custom Post Type Connections

* cleanup per: 76ffd12

* fix location field to enable add new (currently unable to support remove bc mobile app plugin endpoint does not return "grid_meta_id" req for deletion

* fix for "isRequired" translation

* show quick action FAB for contacts only

* enable navigation to comments/activity screen from custom post types

* persist filters by ID

* Icon refinements and additions

* reenable route-based filters

* fix duplicate issue with Locations

* enable custom icon for Alert banner

* use "UpdateRequiredIcon" for consistency

* group connection fields without existing values should behave like typical dropdown sheet

* support newer and older version of available post types in settings

* fix comment bug

* adjust: AddNewIcon + LogsIcon

* svg baptize icons for fields

* SubscriptionsIcon + TrainingsIcon (school)

* Icons for fields about baptism

* support dynamic custom quick action buttons

* auto-open sort sheet to 66% to better ensure all options are displayed

* kebab: My user icons + comment activity

* upgrades: SWR 1.3 for better optimisticUI interface, Expo v45, TypeScript deps

* filter out duplicate requests from offline queue

* see: 782ab87

* Icon changes to work with updated font library

* recommit icon changes after revert, and remove redundant lines

* introduce Slider component for Arrow implementation

* O365 Login and 2FA

* Fix: font icon was renamed in recent update

This fixes the missing icon for the first item in the records FAB

* general refactoring

* update deps

* reintroduce offline support (has known issues!)

* Slider and few bugs resolved.

* prettier definition and husky pre-commit config

* update husky and jest

* prettier test

* re-init husky

* clean up

* run prettier on all source files

* Update store docs

* Update README.md

* Changed protocol to https.

* HTML markup for Comments/Activities

* Removed console logs, implemented separate loading spinners for login, showed baptized details in church health.

* Removed extra space at end of the link

* use placeholder on More screen

* treat More as its own type (to distinguish settings)

* extended language support

* style LogsIcon as placeholder

* options to prevent Image flicker per facebook/react-native#981 ; temporarily remove unused icons

* decode HTML entities in Comments/Activities

* fix spacing issue between comment link and comment text

* bugfix

* swap out deprecated properties

* some addl performance improvements

* fix locations select bug on create new

* fix bottom margin block issue on filterlist

* export asset as PNG (it was actually a JPG renamed as PNG) to pass Play Store build review

* initial version of eas config file

* Dynamic icons for Church Health and Bottom Sheet for icon hint. (#633)

* Dynamic icons for Church Health.

* Removed console logs.

* Updated package-lock.json

Co-authored-by: Steven Shubham <stevenshubham@Administrators-MacBook-Pro.local>

* update expo-notifications package

* support redux migrations for versions that break on persistent state mismatch

* manually update shell-quote package (per dependabot)

* manually update hermes-engine package (per dependabot)

* Sort icons

new and improved sort icons appear in the sort bottom sheet

* Accordion component for activity log (#693)

* Dynamic icons for Church Health.

* Removed console logs.

* Updated package-lock.json

* Accordion component for activity log

* Additional Checks while showing the activity log.

Co-authored-by: Steven Shubham <stevenshubham@Administrators-MacBook-Pro.local>

Resolves #651

* bump version

* Resolves #686 - timestamp to date according to latest PR (#697)

* Fixed Slider issue on Android and minor improvements. (#698)

* Fixes #691 - auto-migrate persisted state when newer version requires it

* general refactor and performance improvements (more memoization)

* update dependency packages per Expo warning

* Fixes #692 (offline cache) and fixes push notification issue

* revert in-app MFA flow in anticipation of switching to web-based login

* Fixes #638

* Resolved O365 redirection issue and broken filters. (#703)

* Dynamic icons for Church Health.

* Removed console logs.

* Updated package-lock.json

* Accordion component for activity log

* Additional Checks while showing the activity log.

* Resolved dark theme issue in AllActivityLogsScreen

* Fixed Slider issue on Android and minor improvements.

* Resolved O365 redirection issue and broken filters.

* Mark all notifications as read.

* Removed LoginScreen, use-auth and package-lock files.

* Resolved Conflicts

* Resolved use-notifications conflict

Resolves #646 and Fixes #532 

Co-authored-by: Steven Shubham <stevenshubham@Administrators-MacBook-Pro.local>

* Resolves #632, clean up rehydration

* Activity,Comment and search screen small bug fixes (#701)

* Resolves #686 - timestamp to date according to latest PR

* Resolve - #699 Resolve - #666 Resolve - #635 Resolve - #566 Resolve - #674 Resolve - #669 activitylog mention date, comment activity mentions and date format, search screen

* resolve conflict

* search bar useeffect typo mistake

* default to expanded accordions

* fix toggle favorites on lists, general refactor and perf improvements

* more reactive response to mark all notifications

* improved language switching

* improved offline experience

* Dev/704 (#707)

* LabeledTextInput refactor

* LoginScreen refactor

* Changing instance of LabeledTextInput on Otp screen

* Refactor Login Screen and LabeledTextInput

Resolves #704

* Html entities decode text in NotificationsScreen (#706)

* update CI/CD (#708)

Co-authored-by: MexSurfer <43966676+mikeallbutt@users.noreply.github.com>
Co-authored-by: Mike Allbutt <yo@ur.id.au>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Steven Shubham <stevenshubham@Administrators-MacBook-Pro.local>
Co-authored-by: steven-shubham <104883753+steven-shubham@users.noreply.github.com>
Co-authored-by: Jackson Harry <jackson.isbd@gmail.com>
Co-authored-by: Maxwell <maxwellmdg@gmail.com>
@zdmc23
Copy link
Collaborator

zdmc23 commented Mar 20, 2023

Resolved by #709

@zdmc23 zdmc23 closed this as completed Mar 20, 2023
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

No branches or pull requests

2 participants