- Issue created by @baluv3
- Assigned to ckrina
- 🇪🇸Spain ckrina Barcelona
I think from Phosphoricons (the library used for the navigation) we could go with this 2 options:
- 🇩🇪Germany rkoller Nürnberg, Germany
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.
- 🇮🇳India aman1248
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.
- Issue was unassigned.
- 🇪🇸Spain ckrina Barcelona
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 → . - 🇮🇳India aman1248
Hy @ckrina,
I have already created MR !239,Please Review It Once.
Thank You. - Status changed to Needs review
3 months ago 12:15pm 11 April 2024 - Status changed to Needs work
3 months ago 12:17pm 11 April 2024 - First commit to issue fork.
- 🇮🇳India adwivedi008
@ckrine @m4olivei
Implemented the suggested changes but got Merge error
Can we resolve this by rebasing with 11.x - Merge request !7854Issue #3418489 by amandeep_lnwebworks, adwivedi008: Choose an icon for the Announcements link → (Closed) created by m4olivei
- Status changed to Needs review
about 2 months ago 6:13pm 30 April 2024 - 🇨🇦Canada m4olivei Grimsby, ON
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!
- Status changed to Needs work
about 2 months ago 2:08pm 1 May 2024 - 🇮🇳India ehsann_95
ahsannazir → made their first commit to this issue’s fork.
- Status changed to Needs review
about 2 months ago 4:58pm 7 May 2024 - 🇨🇦Canada m4olivei Grimsby, ON
Looks like @finnsky's feedback on cleaning up the SVG was addressed. Moving to Needs Review.
- Status changed to RTBC
about 2 months ago 5:01pm 7 May 2024 - 🇨🇦Canada m4olivei Grimsby, ON
I've tested locally. It's looking good!
Marking RTBC.
- Assigned to ckrina
- 🇺🇸United States xjm
We should probably have @ckrina or another subsystem maintainer sign off on the icon choice and the changeset. Thanks!
- Status changed to Needs work
about 2 months ago 6:29pm 8 May 2024 - 🇪🇸Spain ckrina Barcelona
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:
- First commit to issue fork.
- 🇺🇸United States patrickfgoddard
Flipped icon per request (so megaphone is pointing to right). First time using drupalpod, so please excuse if issues with process.
- Status changed to Needs review
about 2 months ago 8:13pm 8 May 2024 - Status changed to Needs work
about 2 months ago 9:19pm 8 May 2024 - 🇪🇸Spain ckrina Barcelona
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 :)
- 🇩🇪Germany rkoller Nürnberg, Germany
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?
- 🇺🇸United States patrickfgoddard
I think I need to rebuild css. Trying to figure out how to do within Drupalpod now.
- Issue was unassigned.
- Status changed to Needs review
about 1 month ago 5:17am 13 May 2024 - 🇮🇳India Kanchan Bhogade
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
- Status changed to Needs work
about 1 month ago 12:37pm 13 May 2024 - 🇺🇸United States smustgrave
Can the issue summary be updated to include what icon was being chosen and possibly why
- 🇩🇪Germany rkoller Nürnberg, Germany
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.
- Status changed to Needs review
about 1 month ago 6:41am 17 May 2024 - 🇮🇳India ehsann_95
The issue was happening due to transform property in the svg itself. Fixed the SVG and moved style to stylesheet
- Status changed to Needs work
about 1 month ago 1:49pm 17 May 2024 - Status changed to Needs review
about 1 month ago 1:51pm 22 May 2024 - 🇨🇦Canada m4olivei Grimsby, ON
Updated the issue description summarizing comments in favor of using the Megaphone.
- Status changed to RTBC
about 1 month ago 9:28am 23 May 2024 - 🇫🇷France nod_ Lille
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!
- Status changed to Fixed
about 1 month ago 1:32pm 24 May 2024 Automatically closed - issue fixed for 2 weeks with no activity.