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

Reorganize system alert messages based on bootstrap alerts #905

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

alvinBM
Copy link
Member

@alvinBM alvinBM commented Feb 16, 2024

No description provided.

@alvinBM alvinBM marked this pull request as draft February 20, 2024 09:07
@alvinBM alvinBM marked this pull request as ready for review February 20, 2024 12:00
@alvinBM
Copy link
Member Author

alvinBM commented Feb 21, 2024

Hello @josaphatim , This PL is ready

@josaphatim
Copy link
Member

In my opinion, the Hm_Notices::show function to which we pass an array of messages as the first parameter should display an alert for each message instead of having a single alert for all the messages passed separated by a comma.
But also in the same function please add the type of error depending on ERR passed before the string or not.

But also the Hm_Utils::show_sys_messages method had the sole responsibility of displaying pending messages already added to the element $('.sys_messages') via elements having the .err class.

I think it could keep the same role but just think about how to keep messages waiting before displaying

@alvinBM
Copy link
Member Author

alvinBM commented Mar 11, 2024

Hm_Notices ins't showing any alert #919

Fixed @josaphatim, Thanks

modules/core/site.js Outdated Show resolved Hide resolved
modules/pgp/site.js Outdated Show resolved Hide resolved
modules/core/output_modules.php Outdated Show resolved Hide resolved
modules/core/site.js Outdated Show resolved Hide resolved
@alvinBM alvinBM force-pushed the sys-messages-revamp branch 4 times, most recently from d52a5af to 288fdc7 Compare March 12, 2024 10:46
@josaphatim josaphatim merged commit 52a978b into cypht-org:master Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants