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

chore: refactored code to use custom hooks #86

Merged
merged 13 commits into from
Oct 4, 2024
Merged

chore: refactored code to use custom hooks #86

merged 13 commits into from
Oct 4, 2024

Conversation

azeezat
Copy link
Owner

@azeezat azeezat commented Sep 9, 2024

Description

Extracted most of the functions in the index.ts file into custom react hooks to improve readability. Feel free to check and comment on this PR. Everything should work as before so if you notice any changes, please drop a comment.

  • Code cleanup

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added unit tests to ensure that the library continues to function as it currently does.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Sorry, something went wrong.

@azeezat azeezat changed the title chore: refactored code to use hooks chore: refactored code to use custom hooks Sep 9, 2024
Copy link

github-actions bot commented Sep 9, 2024

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
95.79% (+39.25% 🔼)
250/261
🟢 Branches
94.31% (+40.91% 🔼)
199/211
🟢 Functions
94.29% (+47.41% 🔼)
99/105
🟢 Lines
96.47% (+39.82% 🔼)
246/255
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / DropdownFlatList.tsx
100% 100% 100% 100%
🟢
... / DropdownSectionList.tsx
93.94% 100% 85.71% 93.75%
🟢 hooks/use-search.ts 100% 100% 100% 100%
🟢
... / use-selection-handler.ts
78.26% 64.29% 80% 81.82%
🟢
... / use-index-of-selected-item.ts
100% 83.33% 100% 100%
🟢 hooks/use-modal.ts 94.12% 75% 100% 100%
🟢
... / use-select-all.ts
100% 100% 100% 100%
🟢 hooks/index.ts 100% 100% 100% 100%

Test suite run success

24 tests passing in 3 suites.

Report generated by 🧪jest coverage report action from 8ae10a7

@azeezat azeezat added the enhancement New feature or request label Sep 9, 2024
@azeezat azeezat requested a review from Tabspear September 9, 2024 10:58
Comment on lines 24 to 28
}, [hideModal]);

useEffect(() => {
if (!open && Platform.OS === 'android') {
onDismiss?.();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@azeezat , i see github action raise coverage issues here?. is this something we can address?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@kofiarkoh Thank you for pointing this out. I just added some tests to address this.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@kofiarkoh How do you currently use the hideModal prop? I did some refactoring on the modal and I haven't been able to find a use case for it

@azeezat azeezat requested a review from kofiarkoh September 28, 2024 07:41
Copy link
Collaborator

@Tabspear Tabspear left a comment

Choose a reason for hiding this comment

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

Clean Code!

Copy link
Collaborator

@Tabspear Tabspear left a comment

Choose a reason for hiding this comment

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

Great Job!

@Tabspear Tabspear merged commit a272f5b into main Oct 4, 2024
6 checks passed
@azeezat azeezat deleted the v2 branch October 19, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants