Conversation
…ose-toast fix: The drawer and the dialog were closing while closing a toaster
Bump version to `1.7.7`
| @@ -1,12 +1,12 @@ | |||
| { | |||
| "name": "@bsf/force-ui", | |||
There was a problem hiding this comment.
What: Version bump in the package-lock.json file should be validated with the actual code changes.
Why: It's important to ensure that version updates reflect genuine changes in functionality or dependencies within the project. Updating the version number without corresponding code changes can lead to versioning confusion when using version control or package managers.
How: Ensure that the corresponding code changes for version 1.7.7 have been thoroughly tested and documented. If there are no changes in the code, consider not changing the version in the package-lock.json until there are meaningful updates.
| "requires": true, | ||
| "packages": { | ||
| "": { | ||
| "name": "@bsf/force-ui", |
There was a problem hiding this comment.
What: Confirm that all dependencies specified in the package-lock file are up-to-date and compatible with version 1.7.7.
Why: Having outdated or incompatible dependencies can open up security vulnerabilities or cause runtime errors within the application. Updating the lock file should ensure that all dependencies match what's currently needed for the version change.
How: Run a dependency check and review the latest versions of each dependency, ensuring all are compatible with the upcoming changes in version 1.7.7.
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@bsf/force-ui", | |||
There was a problem hiding this comment.
What: The version was updated from 1.7.6 to 1.7.7, which is a good practice for versioning, but there is no accompanying documentation or notes in the PR description about any changes in this version.
Why: Proper documentation and notes about version changes help maintain clarity on what has changed and alert users to potentially breaking changes or new features.
How: Consider adding a detailed CHANGELOG file or a section in the PR description to outline what changes or fixes are included in 1.7.7 versus 1.7.6.
| escapeKey: exitOnEsc, | ||
| outsidePress: exitOnClickOutside, | ||
| outsidePress: ( event ) => { | ||
| if ( ! exitOnClickOutside ) { |
There was a problem hiding this comment.
What: Consider adding a check to ensure that event is not null or undefined before accessing event.target.
Why: This will prevent potential runtime errors if the event is not triggered properly, which can lead to unexpected behavior or application crashes.
How: Modify the outsidePress function to include a null check for event like so: if (!event || !exitOnClickOutside) { return false; }.
| enabled: exitOnClickOutside || exitOnEsc, | ||
| escapeKey: exitOnEsc, | ||
| outsidePress: exitOnClickOutside, | ||
| outsidePress: ( event ) => { |
There was a problem hiding this comment.
What: Make sure that isToastElement is correctly typed to avoid potential type issues in TypeScript.
Why: Explicit typing provides better readability and code reliability, which is important for long-term maintenance and understanding of the codebase.
How: Define isToastElement with a proper TypeScript type rather than relying on implicit typing. Consider using: const isToastElement = target?.closest('ul.fui-toast-container') as HTMLElement | null;.
| @@ -119,7 +119,14 @@ const Dialog = ( { | |||
| const dismiss = useDismiss( context, { | |||
There was a problem hiding this comment.
What: The change to outsidePress could be better documented with a comment explaining the logic behind the check for isToastElement.
Why: Comments can improve the understandability of complex logic and help future developers grasp the intent of your code quickly.
How: Add a comment above the outsidePress function explaining the purpose of checking for isToastElement.
Description
Screenshots | Video with voice-over
Link to Figma (If applicable)
How has this been tested?
Checklist: