Problem/Motivation

I would like to show an ordered number and total count on alerts (1 of 3, 2 of 3, etc).

Proposed resolution

Include current number and total as variables to use in the alerts template.

User interface changes

We could add an option to toggle this display.

Issue fork sitewide_alert-3303175

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mortona2k created an issue. See original summary.

mortona2k opened merge request !26

mortona2k’s picture

mortona2k commented
Status: Active » Needs review

This MR adds the necessary variables in the sitewide_alert preprocess and template.

It uses a static variable to count the number of times the preprocess has been called.

Total count comes from SitewideAlertManager->activeVisibleSitewideAlerts().

mortona2k’s picture

mortona2k commented
Status: Needs review » Needs work

I realized there is JS to handle closing alerts and adding new ones. The number handling should go in there so it can get recalculated as needed.

smustgrave’s picture

smustgrave commented
Version: 2.x-dev » 3.0.x-dev

Lets land this in the 3.0.x branch first.

smustgrave’s picture

smustgrave commented

yea lets add a config option for this.

dlevchik made their first commit to this issue’s fork.

dlevchik’s picture

dlevchik commented
Status: Needs work » Needs review

- Merged 3.x into MR
- Added config option for 'Show alerts count'
- Added translation for @current_count of @total_count alerts
- Fixed db queries in a preprocess loop for total count by static caching it
- Added JS function updateAlertCount on initAlerts and removeAlert. Message is dynamic and translatable

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Can you update MR for 3.0.x

dlevchik opened merge request !66

dlevchik’s picture

dlevchik commented
Status: Needs work » Needs review

Created new MR: https://git.drupalcode.org/project/sitewide_alert/-/merge_requests/66

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Nice, lets add some test coverage for this.

dlevchik’s picture

dlevchik commented
Status: Needs work » Needs review

Added tests, it is a FunctionalJavascriptTest. I wanted to test the alerts count change on adding new alert and dismissing another alert, and it kinda become also a test for sitewide_alert_block and refresh_interval. Hope you don't mind such a big test. Besides, there was no test cases for those things before, so it may be to the benefit

Also, a question maybe we may think of: should we hide alerts count when there is only one alert available for user? This also needs to be done via JS I believe, since dismissed alerts are in localStorage

smustgrave closed merge request !26

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Should make them appear inline vs stacked.

smustgrave closed merge request !66

smustgrave opened merge request !88

smustgrave’s picture

smustgrave commented
Version: 3.0.x-dev » 3.1.x-dev

Needed this for a client project but noticed an issue, besides the test failure

When you dismiss an alert and reload the page, 1 alert will show but count is now off.

smustgrave’s picture

smustgrave commented
Status: Needs work » Fixed

Fixed that issue and added additional coverage for it.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

smustgrave’s picture

smustgrave commented
Status: Fixed » Needs work

Not reverting yet but need to make the tests better seem to randomly fail

smustgrave opened merge request !89

smustgrave’s picture

smustgrave commented
Status: Needs work » Fixed

Re-ran a few times and tests pass more consistently

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.