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

fix: Alert unmount and overlay #378

Merged
merged 2 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/components/Navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,12 @@ function Navigation() {
</div>
<Sidebar close={() => setShowSidebar(false)} open={showSidebar} />
</nav>
<div className="new-hathor-alert-container">
<NewHathorAlert ref={alertErrorRef} text="Invalid hash format or address" type="error" />
</div>
<NewHathorAlert
ref={alertErrorRef}
text="Invalid hash format or address"
type="error"
fixedPosition
/>
</>
);
};
Expand Down
94 changes: 67 additions & 27 deletions src/components/NewHathorAlert.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import React, { useRef, forwardRef, useImperativeHandle, useEffect } from 'react';
import React, { useRef, forwardRef, useImperativeHandle } from 'react';
import { ReactComponent as SuccessIcon } from '../assets/images/success-icon.svg';

/**
Expand All @@ -15,7 +15,7 @@ import { ReactComponent as SuccessIcon } from '../assets/images/success-icon.svg
* @param {Object} props - Component properties.
* @param {string} props.type - Defines the alert type for styling (e.g., "success", "error").
* @param {string} props.text - The message text displayed in the alert.
* @param {boolean} [props.fixedPosition=false] - If true, the alert will be fixed at the bottom right of the screen
* @param {boolean} [props.fixedPosition=false] - If true, the alert will be fixed at the bottom center of the screen
* @param {React.Ref} ref - A reference to call the `show` method from parent components.
*
* @example:
Expand All @@ -25,7 +25,8 @@ import { ReactComponent as SuccessIcon } from '../assets/images/success-icon.svg
* alertRef.current.show(2000); // Show the alert for 2 seconds
* ```
*/
const NewHathorAlert = forwardRef(({ type, text, showAlert, fixedPosition }, ref) => {
const NewHathorAlert = forwardRef(({ type, text, fixedPosition }, ref) => {
const containerDiv = useRef(null);
const alertDiv = useRef(null);

/**
Expand All @@ -34,41 +35,80 @@ const NewHathorAlert = forwardRef(({ type, text, showAlert, fixedPosition }, ref
* @param {number} duration - The display duration for the alert in milliseconds.
*/
const show = duration => {
if (alertDiv.current) {
alertDiv.current.classList.add('show');
setTimeout(() => {
alertDiv.current.classList.remove('show');
}, duration);
// If the component is not in a fixed position, only handle the alert component
if (!fixedPosition) {
showAlertComponent();
return;
}
};

useEffect(() => {
if (showAlert === undefined) {
// No-op if the container is unavailable
if (!containerDiv?.current) {
return;
}
if (showAlert) {

// The component is in a fixed position: managing its parent container to allow for animations
// and precise positioning
containerDiv.current.classList.add('show');
setTimeout(showAlertComponent, 100); // Delay the alert display to accomodate animations
setTimeout(() => {
// By the time the timeout is called, the container may have been unmounted
if (containerDiv?.current) {
containerDiv.current.classList.remove('show');
}
}, duration + 500);

/**
* Handles the Alert component display and removal
*/
function showAlertComponent() {
// No-op if the ref is unavailable
if (!alertDiv?.current) {
return;
}

alertDiv.current.classList.add('show');
} else {
alertDiv.current.classList.remove('show');
setTimeout(() => {
// By the time the timeout is called, the component may have been unmounted
if (alertDiv?.current) {
alertDiv.current.classList.remove('show');
}
}, duration);
Copy link
Member

Choose a reason for hiding this comment

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

You don't have this duration parameter here, if I'm not wrong. The diffs are a bit confusing but it seems like you should be receiving it as a parameter in the showAlertComponent also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is actually declare inside the show() function, so it shares the duration parameter.
The diff visualizarion is confusing, since some of the methods were obsolete and were removed, but you can see it more clearly on the file itself.

}
}, [showAlert]);
};

useImperativeHandle(ref, () => ({
show,
}));

const fixedPositionClass = fixedPosition ? 'fixed-position' : '';
return (
<div
ref={alertDiv}
className={`new-hathor-alert alert alert-${type} alert-dismissible fade ${fixedPositionClass}`}
role="alert"
style={{ display: 'flex', flexDirection: 'row' }}
>
<div className="success-icon">{type === 'success' ? <SuccessIcon /> : null}</div>
<p className="success-txt">{text}</p>
</div>
);
/**
* Renders the alert component
* @returns {Element}
*/
function renderAlertDiv() {
return (
<div
ref={alertDiv}
className={`new-hathor-alert alert alert-${type} alert-dismissible fade`}
role="alert"
style={{ display: 'flex', flexDirection: 'row' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to avoid this style from the previous implementation, but the fade animations just wouldn't work without it. Decided to leave this cleanup to a future refactor.

>
<div className="success-icon">{type === 'success' ? <SuccessIcon /> : null}</div>
<p className="success-txt">{text}</p>
</div>
);
}

// If the component is in a fixed position, also render its parent container on demand
if (fixedPosition) {
return (
<div ref={containerDiv} className="new-hathor-alert-container">
{renderAlertDiv()}
</div>
);
}

// Render only the alert component
return renderAlertDiv();
});

NewHathorAlert.displayName = 'NewHathorAlert';
Expand Down
12 changes: 5 additions & 7 deletions src/newUi.scss
Original file line number Diff line number Diff line change
Expand Up @@ -952,14 +952,18 @@ nav {
}

.new-hathor-alert-container {
display: flex;
display: none;
position: fixed;
justify-content: center;
align-items: center;
padding: 0px 0px 17px 0px;
bottom: 220px;
width: 100%;
left: 0;

&.show {
display: flex;
}
}

.new-hathor-alert {
Expand All @@ -975,12 +979,6 @@ nav {
justify-content: space-between;
z-index: 9999;
margin: 0;

&.fixed-position {
position: fixed !important;
right: 15%;
bottom: 50%;
}
Comment on lines -978 to -983
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing CSS leftover from #352, obsolete since #377

}

.alert-success-container {
Expand Down
Loading