-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Allow configuring Announcement banner by admin #10951
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10951 +/- ##
============================================
- Coverage 16.60% 16.60% -0.01%
- Complexity 13924 13925 +1
============================================
Files 5730 5731 +1
Lines 508082 508123 +41
Branches 61770 61783 +13
============================================
+ Hits 84388 84392 +4
- Misses 414259 414296 +37
Partials 9435 9435
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@sudo87 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
@blueorangutan package |
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13635 |
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.
code lgtm
@sudo87 can you please add some documentation regarding this, probably somewhere https://github.com/apache/cloudstack-documentation/blob/main/source/adminguide/ui.rst
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.
The floating banner hides the content behind it. I think it should instead be part of the page view/frame (or top global header component) & push the entire UI content down. See the banner on https://apisix.apache.org/ or https://docusaurus.io/ for example, where the announcement banner is pushing the entire content down, not floating on the UI.
The a-alert Ant Design Vue — An enterprise-class UI components based on Ant Design and Vue.js could be used, or a new component could be created.
@blueorangutan package |
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13670 |
@shwstppr a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
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.
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.
@sudo87 CLGTM. Some minor changes would be good to have:
- Add a dummy entry in
config.json
for the banner which is disabled by default. It helps the end user update the configuration directly on the server. - Allow configuring the banner such that it only shows on login screen or after login or both.
@sudo87 Found an issue. |
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 13744 |
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with@blueorangutan ui SystemVM template(s). I'll keep you posted as I make progress. |
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13748 |
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.
lgtm. tested locally. works as expected.
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.
LGTM
Description
This PR enables Admin to configure UI Announcement banner containing message through config.json. HTML based message is supported apart from regular text
It can be configured by defining following section in config.json:
Following types are supported:
To ensure banner is shown only once, enable the following config:
Doc PR: apache/cloudstack-documentation#517
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
New npm dep (dompurify) is included to sanitize the html.