-
Notifications
You must be signed in to change notification settings - Fork 14
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
new-log-viewer: Add NotificationContextProvider
for managing pop-up messages; add pop-ups for errors and remove status bar dummy message.
#84
Changes from 39 commits
61f4b6e
fc5fbc3
4e2ae14
93e4a3d
3a69b71
43b7378
e8ce67a
0df2470
d0ab264
68f9bfd
670cf09
957f5a0
306cc25
0c30d20
83f5007
fc37786
18c75e4
e346322
d44d073
3a63386
db134b5
729fac6
93ab288
2c92af5
4c51ff2
f4d191a
fe57144
d8d997d
4704393
7881cb5
ddbb577
4635798
d81e258
62409b0
7b71ac4
779d93b
143a4bb
2dec93b
38cafbd
189be81
c81d38c
ac0ee35
e107a94
de99ef3
6980c7a
df5be3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
import { | ||
useContext, | ||
useEffect, | ||
useState, | ||
} from "react"; | ||
|
||
import { | ||
Alert, | ||
Box, | ||
CircularProgress, | ||
IconButton, | ||
Typography, | ||
} from "@mui/joy"; | ||
|
||
import CloseIcon from "@mui/icons-material/Close"; | ||
|
||
import { | ||
NotificationContext, | ||
PopupMessage, | ||
} from "../../contexts/NotificationContextProvider"; | ||
import {WithId} from "../../typings/common"; | ||
import {LOG_LEVEL} from "../../typings/logs"; | ||
import {DO_NOT_TIMEOUT_VALUE} from "../../typings/notifications"; | ||
|
||
|
||
const AUTO_DISMISS_PERCENT_UPDATE_INTERVAL_MILLIS = 50; | ||
|
||
interface PopupMessageProps { | ||
message: WithId<PopupMessage>, | ||
} | ||
|
||
/** | ||
* Display a pop-up message in an alert box. | ||
* | ||
* @param props | ||
* @param props.message | ||
* @return | ||
*/ | ||
const PopupMessageBox = ({message}: PopupMessageProps) => { | ||
const {id, level, message: messageStr, title, timeoutMillis} = message; | ||
|
||
const {handlePopupMessageClose} = useContext(NotificationContext); | ||
const [intervalCount, setIntervalCount] = useState<number>(0); | ||
|
||
const handleCloseButtonClick = () => { | ||
handlePopupMessageClose(id); | ||
}; | ||
|
||
useEffect(() => { | ||
if (DO_NOT_TIMEOUT_VALUE === timeoutMillis) { | ||
return () => {}; | ||
} | ||
const intervalId = setInterval(() => { | ||
setIntervalCount((c) => c + 1); | ||
}, AUTO_DISMISS_PERCENT_UPDATE_INTERVAL_MILLIS); | ||
|
||
return () => { | ||
clearInterval(intervalId); | ||
}; | ||
}, [ | ||
timeoutMillis, | ||
handlePopupMessageClose, | ||
]); | ||
|
||
const color = level >= LOG_LEVEL.ERROR ? | ||
"danger" : | ||
"primary"; | ||
|
||
let percentRemaining = 100; | ||
if (DO_NOT_TIMEOUT_VALUE !== timeoutMillis) { | ||
const totalIntervals = timeoutMillis / AUTO_DISMISS_PERCENT_UPDATE_INTERVAL_MILLIS; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking JoyUI can deal with decimal points I remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would leave it in. In the percent remaining calc |
||
percentRemaining = 100 - (100 * (intervalCount / totalIntervals)); | ||
if (0 >= percentRemaining) { | ||
setTimeout(() => { | ||
handlePopupMessageClose(id); | ||
}, 0); | ||
} | ||
} | ||
|
||
return ( | ||
<Alert | ||
className={"pop-up-message-box-alert"} | ||
color={color} | ||
variant={"outlined"} | ||
> | ||
<div className={"pop-up-message-box-alert-layout"}> | ||
<Box className={"pop-up-message-box-title-container"}> | ||
<Typography | ||
className={"pop-up-message-box-title-text"} | ||
color={color} | ||
level={"title-md"} | ||
> | ||
{title} | ||
</Typography> | ||
<CircularProgress | ||
color={color} | ||
determinate={true} | ||
size={"sm"} | ||
thickness={2} | ||
value={percentRemaining} | ||
> | ||
<IconButton | ||
className={"pop-up-message-box-close-button"} | ||
color={color} | ||
size={"sm"} | ||
onClick={handleCloseButtonClick} | ||
> | ||
<CloseIcon/> | ||
</IconButton> | ||
</CircularProgress> | ||
</Box> | ||
<Typography level={"body-sm"}> | ||
{messageStr} | ||
</Typography> | ||
</div> | ||
</Alert> | ||
); | ||
}; | ||
|
||
export default PopupMessageBox; |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||||
.pop-up-messages-container-snackbar { | ||||||||||||||||
/* Disable pointer events on the transparent container to allow components underneath to be | ||||||||||||||||
accessed. */ | ||||||||||||||||
pointer-events: none; | ||||||||||||||||
|
||||||||||||||||
right: 14px !important; | ||||||||||||||||
bottom: var(--ylv-status-bar-height) !important; | ||||||||||||||||
|
||||||||||||||||
padding: 0 !important; | ||||||||||||||||
|
||||||||||||||||
background: transparent !important; | ||||||||||||||||
border: none !important; | ||||||||||||||||
box-shadow: none !important; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
.pop-up-messages-container-stack { | ||||||||||||||||
scrollbar-width: none; | ||||||||||||||||
overflow-y: auto; | ||||||||||||||||
height: calc(100vh - var(--ylv-status-bar-height) - var(--ylv-menu-bar-height)); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
.pop-up-message-box-alert { | ||||||||||||||||
/* Restore pointer events on the pou-up messages. See above `pointer-events: none` in | ||||||||||||||||
`.pop-up-messages-container-snackbar`. */ | ||||||||||||||||
pointer-events: initial; | ||||||||||||||||
padding-inline: 18px !important; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
.pop-up-message-box-alert-layout { | ||||||||||||||||
width: 300px; | ||||||||||||||||
} | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider a more responsive approach to width setting. While a fixed width of 300px might work well for many screens, it could lead to layout issues on very small or large displays. Consider using a more responsive approach:
Here's an example: .pop-up-message-box-alert-layout {
- width: 300px;
+ width: 90vw;
+ max-width: 300px;
} This change ensures the alert box is never wider than 300px but can shrink on smaller screens. 📝 Committable suggestion
Suggested change
|
||||||||||||||||
|
||||||||||||||||
.pop-up-message-box-title-container { | ||||||||||||||||
display: flex; | ||||||||||||||||
align-items: center; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
.pop-up-message-box-title-text { | ||||||||||||||||
flex-grow: 1; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
.pop-up-message-box-close-button { | ||||||||||||||||
/* stylelint-disable-next-line custom-property-pattern */ | ||||||||||||||||
--IconButton-size: 18px !important; | ||||||||||||||||
|
||||||||||||||||
border-radius: 18px !important; | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import {useContext} from "react"; | ||
|
||
import { | ||
Snackbar, | ||
Stack, | ||
} from "@mui/joy"; | ||
|
||
import {NotificationContext} from "../../contexts/NotificationContextProvider"; | ||
import PopupMessageBox from "./PopupMessageBox"; | ||
|
||
import "./index.css"; | ||
|
||
|
||
/** | ||
* Displays popups. | ||
* | ||
* @return | ||
*/ | ||
const Popups = () => { | ||
const {popupMessages} = useContext(NotificationContext); | ||
|
||
return ( | ||
<Snackbar | ||
className={"pop-up-messages-container-snackbar"} | ||
open={0 < popupMessages.length} | ||
> | ||
<Stack | ||
className={"pop-up-messages-container-stack"} | ||
direction={"column-reverse"} | ||
gap={1} | ||
> | ||
{popupMessages.map((message) => ( | ||
<PopupMessageBox | ||
key={message.id} | ||
message={message}/> | ||
))} | ||
</Stack> | ||
</Snackbar> | ||
); | ||
}; | ||
|
||
export default Popups; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,5 +15,4 @@ | |
|
||
.status-message { | ||
flex-grow: 1; | ||
padding-left: 8px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider increasing the update interval for better performance.
The
AUTO_DISMISS_PERCENT_UPDATE_INTERVAL_MILLIS
constant is set to 50ms, which might lead to frequent re-renders. Consider increasing this value to reduce the update frequency while still maintaining a smooth visual effect.For example, you could increase it to 100ms or 200ms:
This change would reduce the number of updates by a factor of 4, potentially improving performance without significantly affecting the user experience.
📝 Committable suggestion