Implemented a click-outside closing mechanism for the settings window using event bubbling instead of useEffect, to maintain consistency with existing code style.
This approach allows the NavWrapper to automatically close when users click outside its boundaries.
Details
Details
- Reviewers
emack bytesofman - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project
Tested on Chrome and Safari browsers across various screen sizes, with the navigation menu closing effect working smoothly.
npm test passes.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- alitayin
- Lint
Lint Skipped - Unit
No Test Coverage - Build Status
Buildable 32603 Build 64694: Build Diff cashtab-tests Build 64693: arc lint + arc unit
Event Timeline
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.
Tail of the build log:
npm audit fix Run `npm audit` for details. > ecash-lib@3.0.0 build > tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist /work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests added 364 packages, and audited 367 packages in 2s 60 packages are looking for funding run `npm fund` for details 6 vulnerabilities (5 moderate, 1 high) To address all issues, run: npm audit fix Run `npm audit` for details. > ecash-agora@2.0.0 build > tsc && tsc -p ./tsconfig.build.json /work/cashtab /work/abc-ci-builds/cashtab-tests npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options. added 1494 packages, and audited 3350 packages in 25s 328 packages are looking for funding run `npm fund` for details 9 vulnerabilities (8 moderate, 1 critical) To address issues that do not require attention, run: npm audit fix To address all issues (including breaking changes), run: npm audit fix --force Run `npm audit` for details. > cashtab@3.17.4 build > node scripts/build.js Creating an optimized production build... Treating warnings as errors because process.env.CI = true. Most CI servers set it automatically. Failed to compile. [eslint] src/components/App/App.tsx Line 5:39: 'useEffect' is defined but never used @typescript-eslint/no-unused-vars Build cashtab-tests failed with exit code 1
Comment Actions
Implemented a click-outside closing mechanism for the settings window
where is this implemented? I'm not seeing event bubbling above, only the removal of the redundant useEffect.
Comment Actions
Please prefix your title with [cashtab] so we now in the review queue what part of the code is changed
Comment Actions
This is a update.
Perhaps my update method wasn't quite right? It created a new sub-diff, this is the initial submission https://reviews.bitcoinabc.org/D17756?id=52941
Comment Actions
You need to squash your commits and run arc diff again so we have all the changes showing up
| cashtab/src/components/App/App.tsx | ||
|---|---|---|
| 132 | back out and avoid unnecessary linebreaks | |
| 389 | what propagation is this actually preventing? You'd normally use this if you're trying to avoid the burger menu closing when clicking on a menu option, which is the opposite of what you're intending with this diff. In fact, if you back out the changes between line 386 - 391 here, this diff still works as intended. Your onclick logic for CustomApp already achieved the objective. | |