Project messaging channel in core (as experimental)

This could use the Aggregator module.

Did initial push from the module's repository:
Commit id: 176b3cb6cbe70799a208a63da0152f8d81796ee2

Adding screenshots of the current module.

In the red dot image, the bell has a background that is different from the actual toolbar background, is that a current screenshot? The non red-dot screenshots don't seem to have that.

1. Also looks like this feature is encapsulated in a module which would allow people to disable it. There I noticed this needs to be fixed in the info.yml:

description: Module for notify latest updates from drupal community

That should be more like:

description: Notifies site users about latest updates from the Drupal community

There may be more text to review, I only noticed this because I looked at how its enabled and whether it can be disabled.

2. I also noticed that this feature is entirely opt-in (not added to either install profile as far as I see). Is that intentional for this step or as a final solution?

3. Re "determine if we need to start as experimental, or can skip experimental since it is not a site-feature" from the issue summary, this is a site feature. Admin facing features are still site features? The question is how much would you expect the API to change, or if at all there is an API even.

For those interested in testing the Announce module on Drupal 9.3.x,
I created a snapshot, using DrupalPod -

• Using issue fork - 3206643-project-messaging-channel
• Latest commit #854ba58c
• Installed Standard profile
• Enabled Announce module

Username: admin
Password: admin

Click on the following link to open it -
When you see Gitpod's login screen, choose "Continue with Github".
https://gitpod.io/#snapshot/31d89281-104e-4394-85fb-acc624f5e0f0

Based on #3052002: [meta] Replace JQuery with vanilla Javascript in core, we should avoid using jQuery in new code we're adding to core, vanilla JavaScript is probably sufficient.

Accessibility -
There is not enough color contrast in the content of announcements.

The "Announcement" component that displays and hides a dialog, should let screen-readers know about visual change, more details here - https://adrianroselli.com/2020/05/disclosure-widgets.html

I'm concerned about the machine-name of the module: announce.

• In Composer, this is going to be known as drupal/announce.
• In *.info.yml files, it will be known as drupal:announce in the dependencies section.
• The new module introduces new libraries called drupal.announce.dialog and drupal.announce.toolbar.

Meanwhile, there is already a library in core drupal.announce, which provides a Javascript API called Drupal.announce. (It's a clumsy-yet-effective tool for when there isn't a better way to convey important changes to screen reader users.)

I'm not sure if this is a hard namespace conflict which will break things. However I'm sure it amounts to a soft namespace conflict which conflates two separate core features. I expect this will lead to confusion and misunderstanding with respect to documentation/training materials, developer experience, site builder experience, and the Drupal blogosphere generally.

So I'd propose that this module needs to be renamed before being added to core. I don't think it's practical to rename the existing library because of backwards-compatibility commitments.

Re. #26: Beware of conflating dialogs and disclosure widgets. These are not the same thing at all. You mention a dialog, but the linked article does not.

Which behaviour is it: a control which invokes a dialog, or a disclosure widget? That's going to make a difference to how the change of content is conveyed.

I share the concerns about the colour contrast. We'll need measurements, but I'm pretty sure these look poor.

Only one of the messages has an associated "learn more" link. Why don't the others have this? I'm pretty sure I'd want to learn more about the security releases, say.

Some accessibility stuff...

The icons will need to be adaptable to forced-colors modes (previously a.k.a. Windows high-contrast mode). So far, we don't have any multi-colour icons in the toolbar. It's going to need a monochrome representation too.

The screenshots have a lot of dots. What is a dot supposed to signify? It's too vague I think.

The "learn more" link looks like a button, but it behaves as a link. It would be better if it looked like a link, namely underlined text rather than a 4-sided border.

The "learn more" link has _target="blank" which is a massive pain-in-the-accessible, especially since there is no warning that it will open a new window. It fails WCAG SC 3.2.5 Change On Request (level AAA). We really-really-really need to minimise the use of this attribute. In this case, let's just remove it.

I haven't done any other accessibility review yet, or even used the module, so this comment doesn't amount to sign-off.

The templates need documentation

Re #27 Only one of the messages has an associated "learn more" link. Why don't the others have this? I'm pretty sure I'd want to learn more about the security releases, say.The one with the sticky key as true will be acting as a featured announcement and that will have a learn more button. Whereas the other one (the one in white) instead of the learn button the title(link) will be clickable.

One big concern I have here is that this would introduce a whole new pattern in the form of the dialogue/widget/whatever-you-call-it that drops down from the "Announcements" toolbar button (I'm going to just call it "new component" in this post for simplicity).

This new component introduces a new CSS and some JavaScript, this means additional user interface elements to maintain. Core already provides the off-canvas dialogue, and I think it would be better if that were to just be used here.

The off-canvas dialogue could probably also use some love, but it does not make sense to introduce an entirely new component into Core, with its own separate CSS/JS, when what we already have a suitable component ready to be used.

The existing off-canvas dialogue has already gone through accessibility compliance and is already well integrated with Core and contrib. The idea that we should then introduce another, very similar component, with its own design, which requires its own separate long-term maintenance, makes me nervous. We should be aiming for consistency in our Core UI, not divergence.

Thanks,
-Aaron

The module is using drupal off-canvas dialogue. See https://git.drupalcode.org/project/announce/-/blob/1.x/announce.module

The module is using drupal off-canvas dialogue. See https://git.drupalcode.org/project/announce/-/blob/1.x/announce.module

Oh thanks, that's interesting, from the screenshots in the IS it didn't look like it.

The question remains though about a lot of custom CSS. Was there a conscious decision taken not to use the default styling of the off-canvas dialogue? I'm still thinking about the fact that it's basically introducing a completely different visual style here which is unique to this particular implementation.

I opened an issue in the module queue, otherwise i think it'll be very hard to keep track of all the feedback #3233439: JS fixes. In short: JS needs some work, using dialogs API directly, not using jquery when it's not needed, using data attributes instead of classes.

Some feedback not captured in the opened issue:

1. I would expect some validation on the link to make sure it's a Drupal/DA controlled domain that it points to. To make sure that if/when the feed itself is compromised we don't send people to a malicious URL.
2. The experience is a bit jarring, clicking several times on the announcement link makes the bar "flash" because it's making a new request instead of updating what's in there (or not doing anything), probably needs some discussion around what we expect to happen. I would go for several clicks = toggle the sidebar (without refreshing) to keep thing in line with the other buttons of the toolbar
3. The icon looks fuzzy to me, it's too small to have shading (or have several version based on display density). Just noticed it's a png, any reason why it's not a svg as well?
4. There is a cache by user but if I dismiss a featured announcement it just comes back on the next page reload. I would expect the announcement to not be shown or be shown as a non-featured item. Or maybe it's a feature that it's very visible all the time
5. The contrast ratio of the "normal" updates probably doesn't pass accessibility guidelines. It's hard to read for me at least
6. The style is not consistent between the different theme, on claro and olivero there are difference in spacing, alignement, and even the style of the close button
7. For the /announcement page, it should probably be rendered in the admin theme and not the frontend theme no? The announcement toolbar button should probably also be hidden when on that page. There are style missing for the focused close button when viewed as a page
8. Thinking about it we should probably remove the close button altogether from the focused announcement, it doesn't do anything, what is needed is a way to downgrade the focused announcement to a "normal" one after a while to avoid showing the same thing all the time if there aren't many announcements.
9. Focus style are missing for the "Learn more" button, can't see anything when it's keyboard focused

The contrib module is not in sync with this merge request, I'd advise to keep the two very close as I expect there will be a good number of comments and requests for updates. That's what being done for the CKeditor5 module, see #3201824: Roadmap to core and #3231364: Add CKEditor 5 module to Drupal core. The core merge request is generated by a script, i think it'd be a good idea to follow a similar pattern here. Issues that touch on UX always tend to take a while to get in with many discussions, it's easier to manage and follow when each point has it's own issue. (Like the accessibility feedback could have it's own issue to discuss all accessibility related implications of this new UI, without interferring with folks whose main concern is JS or PHP or CSS :)

But so far it's exciting to see this running on a site!

For those willing to see it in action, here's an small gif showing the last code locally:

On a UX perspective, I'm concerned about how to better communicate visually that those are external links. Specially if it's a forced _target="blank" as @andrewmacpherson mentions. An option would be the standard external icon.

On a Design perspective, I think Design should be its own independent point in the remaining tasks list. ;)
1. First, this doesn't follow neither the current Design System Claro is implementing nor Seven's. My guess is this implementation is based on Ana Barcelona's work a while ago and the designs have evolved since then. For example, there aren't outlined buttons anymore (like the one on the blue region).
2. Another point is that links should be blue and underlined. Other visual elements should ideally be used to show that that's been "read" instead of lowering the color to gray, like block background color, icons, or just use a different elements instead of a basic link.

3. The bell icon, could be simplified to play better with the rest of the icons. On the left, the current icon. On the right, an example of a simpler icon style (no the same color tough).

On a code perspective:
1. I'd say that we shouldn't be using PNG for icons: it should be SVGs so some accessibility concerns like the ones expressed by Andrew in #27 can be addressed.
2. This is also introducing a lot of opinionated CSS (that, as @AaronMcHale mentions, will need to be maintained) that is not following existing guidelines. How will this look like on a site using Seven? I think @lauriii can actually give way better feedback here, but ideally a module should just introduce the minimum styles to make it work and let each of the admin themes add its own visual styles.
3. As you can see on the GIF I've uploaded, there's an overlap between the title and the close button on narrow screens.

Anyway, thanks for posting this here and the chance to have this feedback round with some code to test already! :)

Re. #29:

The one with the sticky key as true will be acting as a featured announcement and that will have a learn more button. Whereas the other one (the one in white) instead of the learn button the title(link) will be clickable.

Aha, thanks. They don't look like links, which ckrina mentions in #35.

Re. #32:

Would 'announcements' be free of namespace collision? That seems like a small enough change and just as descriptive

That certainly avoids a hard namespace conflict. However I think the soft conflict is still there.

A thesaurus wasn't much help. Broadcast, notice, and update were the best alternatives I could find. Various contrib modules have already used up a lot of possible terms.

Instead of looking for an variation on "announce", could we name it after where the messages come from? Recalling the goal, emphasis mine:

The goal of this initiative is to provide a channel for messaging about the Drupal project

How about "drupal project announcement", "drupal community news", or something along those lines?

Is there any intention for this UI and/or API to convey messages which don't come from drupal.org as community announcements?

Re. #35:

On a UX perspective, I'm concerned about how to better communicate visually that those are external links. Specially if it's a forced _target="blank" as @andrewmacpherson mentions. An option would be the standard external icon.

It would be far preferable if the _target=blank wasn't used here, regardless of whether we want to tell people it's an external link.

External-link icons haven't been particularly consistent over the years, and are sometimes confused with share/send actions. I recall the UK government dropped them in favour of visible text to say it was external.

The new bell icon is excellent. I was also going to suggest using a cut-out instead of overlap. It will adapt to forced-colour modes in monochrome; the bell and circle-badge will be the same colour, with a clear separator line. That's a strong affordance that it's an additional badge applied to the bell.

Do the disks on the right hand side signify anything? I was wondering if they mean new or un-visited. I think text would convey this more clearly: "new" is what forum module uses.

ggVG

Quick small design feedback: I think the icon that would work better to represent the purpose would be a megaphone.

Edit: Be aware the icons from The Noun Project have a license. We should create our own SVG icon.

As nod_ suggested, this issue is going to follow an automated patch-based approach like #3231364: Add CKEditor 5 module to Drupal core see #3238706

Changes fixed on this patch:
1) Add a check to confirm a particular link is generated from a Drupal/DA controlled domain.
2) Module Rename from announce to announcements_feed.
3) Replace jQuery with vanilla JS.

Triggering the test bot

Unable to run this test locally due to its huge loading time. I believe this will fix it.

Changes fixed on this patch:
1) Add a check to confirm a particular link is generated from a Drupal/DA controlled domain.
2) Module Rename from announce to announcements_feed.
3) Replace jQuery with vanilla JS.

Re. #37:

At first glance this icon looked to me like an axe! I think that's a problem with 2-dimensional silhouette icons. Some of the other megaphone icons on the Noun Project page look clearer:

• Some are 3-dimensional. The megaphone's cone shape is apparent.
• Many have some extra lines, the common cartoon way of showing that sound is coming out of it.
• Many have the central part which sticks out from the middle of the megaphone cone.
• A few have a person holding the megaphone. These are verb-focused (talk) rather than noun-focused (megaphone).

I don't recall seeing a megaphone icon before. (A very similar-looking icon is a side view of a loudspeaker, which is often used for volume control.)

I don't have hard numbers, but I think the bell icon is a more common icon for "notify" or "message"?

A speech bubble is another icon which could work for announcement.

I think there are a few things that need to be improved before considering a +1. I’m concerned with Claro’s blue and other styles forced into all themes with this CSS.

To give you an idea of what I would expect from this project minimum styles, this is how it would look like reusing the off-canvas already existing styles.

I’ve removed locally all the opinionated CSS in announcements_feed.dialog.css, like colors, and left only paddings/margins and hidden undesired visual elements like the default title pencil icon.
Claro’s visual improvements can be added later as a follow-up, or at least they shouldn't appear when Seven is enabled right now.

Also, it would be great if someone could update the summary with Testing instructions for the patch. I’d like to know how to reproduce the content shown on the summary screenshots:
- How can be added a highlighted announce? I’d love to check how this is shown without Claro styles.
- How can several announcements be added? I’d love to see how it behaves with longer and multilingual text.

Re #56 about the icon: I totally agree on a bell as a better representation for notifications, but the module descriptions says “Notifies site users about latest updates from the Drupal community”. I suggested a speaker because I understood this is not supposed to be a user or site notification feature, but a the community announcements channel.

But reading the Release notes snippet from summary the sentence “important information for site owners on older Drupal version” confuses me:
> Announcements may include: news about upcoming Drupal features, important information for site owners on older Drupal versions, and information about supporting the Drupal project through Drupal Association programs.

If this is supposed to just communicate generic information (not targeted to the current site) I see this type of content different to the site/user communications/notifications. If this is supposed to be integrated at some point as “notifications” (like current site’s updates) maybe it should be something to discuss here. I wouldn't see the Toolbar icon as a blocker at this point, but I think it'd be good to make clearer what this project isn't supposed to be (a site's notification place, if I'm right).

re #57

1. go to the module service file there you can change the url parameter...
2. Since the announcement is tempstored wrt time you might forcefully delete it from the db so run this\Drupal::service('keyvalue.expirable')->get('announce')->delete('announcements'); on your drush php

Fixes form this patch includes:

• 3243253-css-refactoring-for: Fixed css linting errors.
• Make announcements_feed_hook_help() comply with the format expected by Drupal core.
• CSS added to claro and seven theme to accommodate the style

Refer to the gif.

CSS refactoring changes.

Some CSS improvements I'd see here are:

1. +++ b/core/modules/announcements_feed/css/announcements_feed.dialog.css
   @@ -0,0 +1,62 @@
   +  background-image: url(../images/close_announce.svg);
   

   Wouldn't it be better to just reuse/link the existing one in core under /core/misc/icons/bebebe/ex.svg? You can check how Layout Builder is implementing it.

2. +++ b/core/modules/announcements_feed/css/announcements_feed.toolbar.css
   @@ -0,0 +1,14 @@
   +  background-image: url("../images/new-announcement-bell.svg");
   

   Maybe the red circle could be done with the ::after sub-element so there's no need to add another SVG file that looks almost the same.

3. Also, on a UX/behavior perspective about the Toolbar link: the rest of the items on the Toolbar, when they're clicked when the sub-menu is opened (like the Manage item), close the sub-menu. I would expect the same: if the Announce tray is opened when the user clicks on it, it should close the Settings Tray.

I tried to work on the announce module this week but I found it a little hard to make significant progress on this because the high level plans for this project are still a little unclear to me. I've read the ideas queue issue #2940739: Project messaging channel in core initiative but it didn't answer all of my questions. It explains the high level goals, but doesn't really go into much detail on how we are trying to solve that. I'm wondering if some of the discussion that has happened in person has been documented somewhere outside of the issue?

There are couple of issues in the announce module issue queue that are asking clarification on some of the key concepts (#3249644: Clarify behavior of announcements, new, viewed, hiding, etc. and #3249625: Document difference between featured and standard announcements). It would be useful to work on those, and document the outcome in the module, as well as potentially adding a summary to this issue. It's important that there's enough understanding on how this new module is intended to work before it is added to core, to make sure that it can be maintained in long term.

Are we planning to add this module to standard install profile? Are there plans on enabling this module on existing sites? If we are not adding this module to standard install profile, or enabling it on existing sites, how are we getting users to enable this module on their sites? What's the value the users get out of the module that makes them install it?

The style is not consistent between the different theme, on claro and olivero there are difference in spacing, alignement, and even the style of the close button

Is there already a plan how we can make the design consistent across different themes?

To add to what lauriii wrote above it was not clear what type of communication would be pushed thought that module initially.

Would it be possible to have a list of the last 5 communications (just the titles at least) that would have made it in that feed based on the past few months? The communication style of the DA which is very US-like and doesn't work as well in the rest of the world. Are the person/team in charge of approving communications all going to be US-based or will there be a mix of locations?

Possibly a relevant issue for the recent discussion.

Opps forgot to take the latest pull.

Some issues fixed:

#3248673: Toggle announcement dialog on clicking toolbar link.
 #3250089 "Remove close button. 
#3249521: Various documentation fixes 
#3249860: Rename templates according to core convention
#3249629 : Simplify regex in.
#3249535: Config and state keys used are incorrect (still using old module name).
#3249661: Filtering for Drupal version is only done on newly loaded announcements.
 #3249640: Implement BEM in the twigs.
#3250091: Remove duplicate ids

#72 is addressed and resolved.

I tested patch #3206643-82.patch and have the following results

• I can enable module and see announcements feed without any issues
• Only one items is visible with date in 2021, as expected from https://www.drupal.org/announcements.json
• Items without body do not show up in feed
• Extra drush step for is required for publishing so we can review it before something goes to every site.

Looks good to me!

The last patch does not apply to the branch.

@matiasmiranda Thanks for reviewing! It would be better to know what you did to review it and to ensure any criticism listed above has been fixed.

Uploaded a rerolled patch, thanks!

+++ b/composer.lock
--- b/core/modules/announcements_feed/tests/modules/announce_feed_test/src/Controller/AnnounceTestController.php
+++ a/core/modules/system/tests/modules/advisory_feed_test/src/Controller/AdvisoryTestController.php

hmmm is the interdiff wrong ???

Re: #92, the diffstat also does not match the previous patch, so someone should check over the reroll carefully.

We also still need usability and accessibility signoff here before this issue is marked RTBC.

#75 also does not seem to have been addressed yet, and it's unclear to me whether IS updates addressed #74 or not either. It's also not clear to me if #72 has been addressed... etc. In general, when posting new patches or updating the IS, please also indicate which previous reviews are (or aren't) addressed by the update. Thanks!

I've tried to apply the patch from #91 to a fresh install of Drupal 9.5.x-dev@dev and it failed to apply unfortunately. But based on #93 a reroll might be required anyway.

In regards of the UX review I could put it on the agenda of next weeks meeting? It takes place on the 15th of july Friday at 14:00 UTC (currently 7:00am PT, 10:00am ET). Today there won't be a regular meeting it looks like and that would also have been on too short notice for someone to join to present and or for answering questions probably. Because it proofed helpful if someone familiar with an issue could find the time to attend the meeting.

Updating title and summary with new tasks

Are we planning to add this module to standard install profile?

The plan is to add this eventually. This can be handled in a followup issue. First step is to get into core as experimental.

Are there plans on enabling this module on existing sites?

I think this is a great idea, however this needs to happen later in a followup issue.

Updating issue summary.

@Lal_, would it be possible to move to Merge request instead of using patch or it is too complex at this time?

Re-roll of #91 for Drupal 9.5.x. Note that I removed the composer changes because of https://www.drupal.org/node/3295096

Adding @todo item in issue summary:

Figure out if this is realistically going into Drupal 9, or should we just target D10

I reached out to @Gábor Hojtsy in Slack at https://drupal.slack.com/archives/C1BMUQ9U6/p1658422784574099

From that thread

Some committers had concerns about the contents of the feed, governance around that, etc. to be defined before any technology ends up in core. Better/more sample messages. I think @lauriii was vocal about this? If memory serves right.

We need a followup issue for this.

Some committers were concerned about the custom solution to a newfeeds basically, but since we are removing RSS support in core in Drupal 10, its probably what it is. Nevertheless agreement on that would be important. Framework managers would be appropriate for this

This can probably be done in this issue.

This could only land in Drupal 10.1, but that is not too far off, I mean the dev branch possibility. That branch is already open and with the Drupal 10 beta it should be the target for stuff anyway in about 50 days.

We'll need to re-roll this patch for Drupal 10.0.1

There were concerns about the functionality of reading tracking, etc. and how useful is it on the level its implemented now.

We should probably take care of this in this issue.

Does this issue still need usability review? The issue tag is still there, but it was removed from the "Remaining tasks" in the issue summary in #84.

I tested the patch in #99, and it applies cleanly to the current 9.5.x branch. I will cross off that item from the list.

I tested giving a user the permission from this module but not permission to see the Toolbar, and the result is that my test user could not see the notifications. It might be worth mentioning that in the description (help text) for the permission, or it might be a good idea to have more than one way to view the notifications.

We discussed this issue at #3236318: Drupal Usability Meeting 2021-09-24, but it seems that none of us left a comment here to summarize the discussion. That issue has a link to a recording of the meeting.

@benjifisher, does that mean that with patch #99 this issue status can be changed to "Needs review" or it is still "Needs work"?

It is too late to get this issue into Drupal 9.5 or 10.0, so this issue should target 10.1. Since the patch in #99 for 10.1.x failed static analysis, the status should still be NW.

I am attaching a patch based on the one from #99, with merge conflicts resolved for 10.1.x. I am also attaching a diff (not an interdiff, since it is a reroll).

Running the core checks locally, I see the same failures that the testbot reported in #99, so I am asking the testbot to ignore this patch.

The attached patch fixes the problems detected by static analysis. +1 for the changes required by PHPStan.

The changes to the JS file are the result of

yarn run -s lint:core-js-passing --fix modules/announcements_feed/js/announcements_feed.announcement.js

(from the core/ directory).

Back to NR.

Remove all files added to the seven and stable themes.

I figured out why the tests were failing: I am testing with Drupal 10, and each test announcement specifies a version constraint: 8 or 9 or both. The tests expect to find at least one announcement (for the current version) but there are none. I suppose I could have added new test announcements, but instead I promoted 8 ->9 and 9 -> 10.

All the failing tests from #110 now pass locally, so this patch may get past the testbot.

In #112, I fixed the FunctionalJavascript tests that were failing, but my change to the fixtures also broke the Kernel tests. The attached patch fixes those.

Usability review

We discussed this issue at #3236318: Drupal Usability Meeting 2021-09-24, but at that time no one left a comment here. There is a link on that issue to a recording of the discussion.

As I recall, the consensus at that meeting is that it would be better to revive a dashboard: by default, users see the dashboard when they log in, and that would be a better place to show the announcements. Seeing announcements on login may be just as effective as the link in the toolbar, and there would be less clutter in the toolbar.

We discussed this issue again at #3315240: Drupal Usability Meeting 2022-10-21. That issue will have a link to a recording. We still think that a dashboard is the better approach. This time around, there was more support for the toolbar/settings tray approach as an interim solution. There was still some concern that this approach would make it harder to switch to a dashboard in the long run, and that it adds too much clutter to the toolbar.

The main usability problem we identified is that the red dot is not an effective way to indicate that there are new announcements:

• It is generally bad practice for color to be the only indicator.
• The difference in shape is easy to miss.
• The red dot is easy to miss for users with limited vision.

We considered a few alternatives, such as the number of new announcements before the word "Announcements". Using the same colors (white text on black) makes it easy to read, but that does not suggest that the number refers to the count of new announcements rather than the total. We think that putting a circle around the number, or using a light, colored background for the number, would suggest that. This is a common pattern in mail apps for mobile devices, for example.

One advantage of putting the announcements on a dashboard is that there is more room to convey the information that there are new ones.

Since we would like to see the announcements added to a dashboard as the long-term solution, there should be some way to make that happen. I am adding the tag for a follow-up issue: we should have an issue to integrate with Views, or have some other way to create an Announcements block.

Another feature request (can be a follow-up issue) is that we provide an individual option to opt out entirely, or to show the link only when there are new announcements.

We had some other suggestions that are not really related to usability. Some are more the responsibility of the release manager. We were concerned that this module does just one thing, and is not easily extensible. Perhaps the architecture could be changed: separate the job of fetching announcements from the job of adding them to the toolbar and the off-canvas dialogue. One or both of those features might be easier to use for other purposes.

In fact, the job of fetching data from an external feed used to be the job of the Aggregator module. Didn’t we just decide to remove that from Drupal core?

As work continues on this feature, keep in mind extensibility and think about transitioning to a dashboard solution. For example, there has already been discussion of supporting more than one feed. (Is there a separate issue for that yet?) Perhaps the internal data structure (the variables passed to the Twig template, not the structure of the feed) should include some identifier for the feed.

For the record, the attendees at today's usability meeting were @AaronMcHale, @benjifisher, @rkoller, @shaal, @simohell, and @worldlinemine.

Adding ideas issue for modern Dashboard #3244581: Enhance user experience with customizable dashboards.

@benjifisher, thanks for posting meeting notes. It would be good to complete this feature and add to D10, and keep dashboard a separate project. I am not clear if this issue needs work (coding) or it can be tested? Thanks for all the work on this issue!

i've taken a look at MR2889. One detail i've noticed the color contrast for the label New in red has a too low color contrast. The text has a color of red #ff0000/rgb(255,0,0) and a background color of #464646/rgb(70,70,70) which leads to a color contrast of 2,36:1 but 4,5:1 would be required (ref WCAG SC 1.4.3).

in high contrast mode in edge (on macos) the pencil disappears and the closing the sidebar button has also a low color contrast.

and i wonder currently the announcements menu item is a link like the other top level ones but it doesnt have the role button. that way it is listed in the rotor under links instead of form controls. the advantage of having a form control like devel (in case installed), shortcuts or manage. the item is announced with for example shortcuts toggle button. and it would also be possible to announce the change of state for the sidebar like devel does with the secondary menu. on key press it announces:

tray "developement menu" opened
or
tray "developement menu" closed

currently when pressing the announcements link the sidebar title immediately gets announced followed by the first content link. having an announcement about the state might be helpful and slow things down?

@rkoller Did you test this in D9 or D10? From what I recall, the pencil/closing icons are problems with the off-canvas menu that still exist in D9 but are fixed in D10.

woops forgot to mention the version number. sorry. i've tested on 10.1.x-dev. went with the version the issue is targeted to.

and one addition to #119 . done a short screen recording using voiceover to illustrate the points about the toggle button and the announcements.

@mherchel, do we need any additional information? It would be super cool to release it with D10 launch party :)

@mherchel, do we need any additional information? It would be super cool to release it with D10 launch party :)

I refactored the CSS to use PostCSS and use the new custom properties that are now available in the D10 off-canvas dialog. This also fixes the icon not showing in the toolbar when using forced colors.

I still have a couple questions regarding the CSS and HTML, that I asked within comments within the MR above.

In addition to this, we need an issue summary update on

• How to test out all of the functionality. How do I see featured announcements, or when there's no announcements?
• What is this supposed to look like? The figma link above just links to one page that says "Alerts" in the toolbar. I'm guessing this is outdated, because the actual module says "Announcements".

One bug that I noticed: If you close the announcements feed using the ESCAPE key, clicking the button to re-open it does not work.

Hey @mherchel I have been trying to fix the bug you mentioned in #124.

Ended up with changing the `$('.announce-dialog .announce-close').on('click', function` in announcements_feed.announcement.js.

        $('.announce-dialog .announce-close').on('click', function () {
          dialog.open = false;
        });

        $(window).on('dialog:afterclose', function (event, dialog) {
          $('[data-drupal-announce-trigger]').trigger('click');
        });

Its working with a console error

Uncaught Error: cannot call methods on dialog prior to initialization; attempted to call method 'widget'
at Function.error (jquery.min.js?v=3.6.1:2:2616)
at HTMLDivElement. (widget.js:234:16)
at Function.each (jquery.min.js?v=3.6.1:2:3003)
at S.fn.init.each (jquery.min.js?v=3.6.1:2:1481)
at t.fn. [as dialog] (widget.js:224:10)
at Object.getContainer (off-canvas.js?v=10.1.0-dev:302:23)
at resetSize (off-canvas.js?v=10.1.0-dev:207:42)
at later (debounce.js?v=10.1.0-dev:37:23)

Fixed it, pushing it to the issue branch, asap.

For the initial commit as experimental, I think it would be good to have real content already in the feed ready to go, so that the module can pick it up as soon as it's installed. SImilarly, when going from experimental to stable, we'd want to make sure there's fresh/relevant content in the feed too. I don't know what a desirable frequency is, but whatever that is it'll need to be maintained, otherwise it'll look stale/broken.

I haven't actually reviewed the feed parsing (or the code more generally) yet, but I agree with not API-ifying it (or going back to aggregator etc.). If we want to add back an RSS parser to core, we should do that in a separate issue - Zend's parser caused us many, many issues with PHP version support and etc. so I would not want to go back to that.

+++ b/core/modules/announcements_feed/tests/announce_feed/updated.json
+++ b/core/modules/announcements_feed/tests/announce_feed/updated.json
@@ -0,0 +1,47 @@
+[{
+        "id": "201",
+        "title": "new 9 - 10 Drupal 9.1.3 is available",
+        "teaser": "This release will have a community alert prototype to notify site admins about drupal updates and required information",
+        "link": "https://www.drupal.org/project/announce",
+        "sticky": true,
+        "version": "^9 | ^10",
+        "updated": "2021-01-19T07:29:38+00:00"
+},

Its not actually RSS, but a custom JSON format that is also demonstrated in the tests.

Why did we go with a custom format over something like https://www.jsonfeed.org/ ? Even if the feed URL consumed by this module will be hardcoded, using an existing format means it could be more easily parsed by other sites/software, if they wanted to. Drupal is committed to the open web and using a custom format for announcements seems the antithesis of that.

I am unable to identify why the code quality checks are failing, and cannot reproduce it when I run the script locally. Looking at the verbose results on dispatcher, I see this:

16:42:56 yarn run v1.22.19
16:42:56 $ node ./scripts/css/postcss-build.js --check --file /var/www/html/core/modules/announcements_feed/css/announcements_feed.toolbar.pcss.css
16:42:57 [22:42:57] '/var/www/html/core/modules/announcements_feed/css/announcements_feed.toolbar.pcss.css' is being checked.
16:42:58 [22:42:58] '/var/www/html/core/modules/announcements_feed/css/announcements_feed.toolbar.pcss.css' is not updated.
16:42:58 error Command failed with exit code 1.

Could this be some sort of bug with #3308872: Address "postcss.plugin was deprecated" warning? Or a bug in the MR's CSS that leads to it failing to detect the file? The file certainly exists in the MR.

Just recompiled the CSS (there was one change). That may have been why it was failing 🤞

Fixing attribution.

Looks like that was it indeed; thanks @mherchel. So "X is not updated" doesn't mean "I, testbot, have nothing to review"; it means "you updated the built asset and not the source, or vice versa."

I posted #3331818: commit-code-check.sh gives unclear output when CSS is not properly rebuilt for the lack of clear debugging info from the commit check script for PostCSS linting.

Why did we go with a custom format over something like https://www.jsonfeed.org/ ?

The simple format was made to match the requirements. I was not aware of JSON Feed. It took me a minute to find the spec, here it is: https://www.jsonfeed.org/version/1.1/.

We do have at least one custom element, what version constraint the message applies to. JSON Feed's spec does have an Extensions section, so that would work.

Not much else is really required by the spec, so it would be workable, if we want to go this direction.

There's a module that adds a view display plugin for json_feed https://www.drupal.org/project/json_feed with 300 users. That suggests at least some people are using json_feed with Drupal so given the change would be purely renaming some keys in the custom format to match the spec (hopefully?), that seems like a good change.

I just provided feedback on the sample real first post in the google doc. In particular I think we need to explain what kind of data is collected (if any) about users of the project and how to opt out. Not necessarily all fully explained in the post but at least linked from it. See my comments in the doc https://docs.google.com/document/d/1d5Uq1ORQDy5bW_3WAyyorEVuHCtZBBB_aKMt...

I updated the issue description with testing instructions on a drupalpod instance with the latest changes and reading from the json feed from the test files. It's here: https://gitpod.io/#snapshot/828d66df-5e7e-41bf-8a32-46e84e2a9f1f (then open browser and go to the /admin/announcements_feed screen) and further details on how to change the feed or trigger the notifications as new are provided in the testing section.

Not sure what's next, who should review this or what else if needed. I'm happy to jump in wherever it's needed.

Follow up from #153:

• ✅ Color of the red button - fixed and waiting on final review confirmation
• ✅ Tests were not passing (still running while I'm writing this)
• ✅ Demo needs to show more items - it points to the test data.
• ❌ First post feedback needs to be resolved - still outstanding, assigned to @hestenet
• ✅ Json format needs to be refactored: refactored here on this MR and #3336530: Make announcements json feed format consistent with https://www.jsonfeed.org/version/1.1/. pending review and deployment.

https://www.drupal.org/announcements.json now has the placeholder in the jsonfeed format. I think the next most important thing is to get real content into the feed, updated on a regular basis, as if this were in production.

✅ Demo needs to show more items - it points to the test data.
- Reviewer: @catch

Unless I'm missing something, I don't think #129 is addressed.

I can still only see one post on https://www.drupal.org/announcements.json - it is saying that something is about to be added to core.

If this is merged into core as experimental, then we do not want an announcement that something is going to be added to core, because it'll already be in core, so the information will be at best outdated, at worst confusing/misleading.

The feed should have actual relevant announcements for someone who would test the experimental module, before it's committed, so that they're immediately available once it is. And then part of the process of getting this to stable/adding it to standard/umami would be that those announcements continue to be up-to-date on whatever we think a reasonable schedule is. It's the first time we've added something to core that has an actual content dependency on Drupal.org, except maybe update status in a way, and once it's on sites, unless they uninstall it, core releases can't do anything about the contents of the feed.

Reviewed the suggested first post in the google doc, I still have doubts about the survey, see there. I made a couple suggestions to parts that were not yet done.

✅ Json format needs to be refactored: refactored here on this MR and #3336530: Make announcements json feed format consistent with https://www.jsonfeed.org/version/1.1/.
- Reviewer: @longwave

The changes for this on both sides mostly look good. However, reading the JSON Feed extensions spec again I have a couple of comments:

For instance, imagine a podcast directory service — call it Blue Shed Podcasts — that asks a podcaster to specify some additional information about a feed or item. A publisher would do something like this:

"_blue_shed": {
    "about": "https://blueshed-podcasts.com/json-feed-extension-docs",
    "explicit": false,
    "copyright": "1948 by George Orwell",
    "owner": "Big Brother and the Holding Company",
    "subtitle": "All shouting, all the time. Double. Plus. Good."
}

The about string is there for a human looking at the feed, so they can understand what goes in the custom extension. It’s optional, but it should appear at least once in a feed that may contain multiple uses of _blue_shed (preferably in the first use, but that’s not required).

Also, it’s good practice to name an extension with a company or service name, to provide a clue right away as to what it’s for and who made it.

Our extension is currently named _extra which isn't very descriptive. I think the extension should be named _drupal or _drupal_org (*not* _drupal.org, as that is disallowed) and we should consider an about key that links to a relevant URL - I feel strongly about the former but not so strongly about the latter; we can't easily change the extension key later, but we could add the about element at any time in the future.

Addressed #162 here https://www.drupal.org/project/drupalorg/issues/3336530#comment-14912886 . I named it "_drupalorg" as that's more consistent with the naming we use in the code all over, but happy to change it as needed.

Thanks for the swift update - _drupalorg is better if that's consistent, no further comments from me on the JSON Feed side of things.

The issue with the red text being inaccessible is fixed (the red was removed)

One more issue that I noticed:

The date is currently formatted MM/DD/YYYY. However that's an americanism. Most of the world (I believe) uses DD/MM/YYYY. In Olivero, we sidestepped the issue by having a date like " 2 April, 2021".

We should also remove the leading zero from the time, and add AM/PM etc.

Thoughts on all of this?

I support 100% using complete name for the month, not two digits, like " 2 April, 2021". @fgarlin, how difficult would it be make this change?

The date is currently formatted MM/DD/YYYY. However that's an americanism. Most of the world (I believe) uses DD/MM/YYYY.

It seems to me that the pattern of the date format (currently "short") depends on the UI settings. Sites that have this set up to DD/MM/YYYY should have also this date formatted the same:

  <div class="announcement__date">{{ announcement.timestamp | format_date('short') }}</div>

Correct, it will adapt to the set up of the site, following the "short" date format. So nothing to do on that end I assume.

Have we established any governance or process around what messages will appear in the feed?