Choose an icon for the Announcements link

@ckrina bump

Thanks! I'll work on this.

I think from Phosphoricons (the library used for the navigation) we could go with this 2 options:

big +1 for the megaphone. bells are about getting your attention and notifying you about a certain event plus a bell icon is the most common image used in the context of notifications across the web as well as operating systems. while a megaphone you are are using to "tell something to the world" which is happening here, the drupal association is announcing certain details.

Thanks for the suggestion,@ckrina @rkoller! I completely agree that the megaphone better captures the essence of making announcements, aligning perfectly with the purpose of our navigation. Your insight regarding the common association of bells with notifications further solidifies the choice for the megaphone.

I've gone ahead and implemented the megaphone icon for the announcement, creating a patch to reflect this change. Additionally, I'll be submitting a merge request to ensure this improvement is seamlessly integrated.

Thanks all for the feedback, un-assigning then to let others contribute here. The the icon to implement this changed can be taken directly from Phosphor and adapted. https://phosphoricons.com/?q=%22megaphone%22

@amandeep_lnwebworks thanks for working on this! We don't use patches anymore, we use to contribute MRs :)
You can read how the workflow works at Creating merge requests → and a broader guide in Using GitLab to Contribute to Drupal → .

Hy @ckrina,
I have already created MR !239,Please Review It Once.
Thank You.

@ckrine @m4olivei

Implemented the suggested changes but got Merge error
Can we resolve this by rebasing with 11.x

m4olivei → closed merge request !239

Thanks @adwivedi008!

The navigation module moved to core rencently in ✨ Add the new Navigation to core as an Experimental module Fixed , so we need to move this MR to be against Drupal core.

I've gone ahead and done that. Unfortunately there was no easy git way to move the commits since they were against two different repos.

Marking this as Needs Review. For me it's looking good!

I've added comment in MR

Looks like @finnsky's feedback on cleaning up the SVG was addressed. Moving to Needs Review.

I've tested locally. It's looking good!

Marking RTBC.

We should probably have @ckrina or another subsystem maintainer sign off on the icon choice and the changeset. Thanks!

Oopsie, did not mean to remove the event tag.

Sorry, small detail! The icon megaphone should be pointing to the other direction, so moving this again to Needs work:

Wrong:

It should actually be:

Flipped icon per request (so megaphone is pointing to right). First time using drupalpod, so please excuse if issues with process.

Would it be possible to clean-up&minimize the SVG itself too through a tool like https://jakearchibald.github.io/svgomg/? Sorry I didn't see it sooner :)

hm i've applied MR7854 and on ltr the announcement icon is still facing to the left for me? and talking of the reading direction. if the icon should point to the right for ltr, should then the icon point to the left for rtl?

I think I need to rebuild css. Trying to figure out how to do within Drupalpod now.

Hi
I've tested MR !7854 on Drupal 11.x
MR is applied cleanly...

The Megaphone icon is added for the Announcements link with the correct direction.

Adding SS for reference

RTBC+1

Can the issue summary be updated to include what icon was being chosen and possibly why

I've applied the latest changes. When I test in Edge in LTR the megaphone points to the right while on RTL i can confirm it points to the left - that is the expected behavior. BUT when i test in Safari (17.4.1 on macOS 14.4.1) it is the other way around, LTR still points to the left and RTL points to the right.

The issue was happening due to transform property in the svg itself. Fixed the SVG and moved style to stylesheet

#36 still needs to happen.

Updated the issue description summarizing comments in favor of using the Megaphone.

MR looks good and S has been updated as requested in #36.
Marking as RTBC.

Committed and pushed ebb6a6e331 to 11.x and bf6a394c50 to 11.0.x and 67178dbf06 to 10.4.x and 02b12c1e11 to 10.3.x. Thanks!

nod_ → closed merge request !7854

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