Problem/Motivation

Since it's now "X" (formerly known as Twitter), we need a new icon in the `rrssb-plus` library.

CommentFileSizeAuthor
#11 update-icon-for-x-twitter-3408538-11.patch602 bytesmaxmendez
#8 Screenshot 2024-01-22 at 16.24.19.png7.95 KBbram.velthoven
#7 Screenshot from 2023-12-19 15-41-50.png16.99 KBsamir_shukla
#3 3408538.patch556 bytessamir_shukla

Issue fork rrssb-3408538

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

JCL324 created an issue. See original summary.

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

samir_shukla’s picture

samir_shukla
Primary language English
Location bareilly
commented
Status: Active » Needs review
StatusFileSize
new 3408538.patch556 bytes

Hi, created the patch for the issue. Please review

Status: Needs review » Needs work

The last submitted patch, 3: 3408538.patch, failed testing. View results

samir_shukla’s picture

samir_shukla
Primary language English
Location bareilly
commented
Status: Needs work » Needs review

Hi, the pr https://github.com/AdamPS/rrssb-plus/pull/14 has been created for rrssb-plus to add icon attached to this comment for X. Once its merged and rrsb-plus is updated, the patch should work fine. Please review again after pr is merged.

sourabhjain’s picture

sourabhjain
Primary language English
commented

@samir we have updated the svg icon in rrssb-plus git repo and it is merged by @adam at https://github.com/AdamPS/rrssb-plus/pull/14.
Please check the changes now.

samir_shukla’s picture

samir_shukla
Primary language English
Location bareilly
commented
StatusFileSize
new Screenshot from 2023-12-19 15-41-50.png16.99 KB

Hi, the pr is merged of that icon on rrssb-plus library, but when installed via composer the icon is not visible in that library. This is the screenshot when we manually added the X svg related files.

bram.velthoven’s picture

bram.velthoven commented
Version: 8.x-2.x-dev » 8.x-2.5
Status: Needs review » Needs work
StatusFileSize
new Screenshot 2024-01-22 at 16.24.19.png7.95 KB

patch needs work: I have tested the patch on version 8.x-2.5 and the results are not as expected, i've configured the new 'X' option in the module settings form but when i go to check out the icons at the frontend there is no X icon.

bram.velthoven’s picture

bram.velthoven commented
Version: 8.x-2.5 » 8.x-2.x-dev
adamps’s picture

adamps commented

Yes if we just change the array key from twitter to x then it will break existing sites. It would need an update hook.

Or maybe some sites will want X and some Twitter? We could keep both options??

maxmendez’s picture

maxmendez commented
StatusFileSize
new update-icon-for-x-twitter-3408538-11.patch602 bytes

I believe it would be better to allow both options—support for Twitter and X. This way, site maintainers can choose to use either platform or both, depending on their needs. It would also allow for a smoother transition, letting those who still refer to Twitter gradually shift to X.

Here is a patch that adds settings for X and is compatible with the dev version of AdamPS/rrssb-plus, which includes the X icons

adamps’s picture

adamps commented

Thanks. Drupal.org no longer supports patches, only merge requests please.

maxmendez opened merge request !18

adamps’s picture

adamps commented

Thanks. The tests are failing because I need to create a release for RRSSB library.

adamps’s picture

adamps commented

I created the RRSSB library release. I tried to rerun the tests, but it still picked up the old version. Maybe I need to wait longer, or somehow clear a cached download?

maxmendez’s picture

maxmendez commented

I've rescheduled the build, and Composer has now picked up the new library version. I also rescheduled the PHPUnit tests, and it looks like they passed successfully.

  • adamps committed a3da4aa0 on 8.x-2.x authored by maxmendez 
    Issue #3408538 by maxmendez, samir_shukla, adamps: Updated icon for X/...
adamps’s picture

adamps commented
Status: Needs work » Fixed

Great thanks everyone, now fixed

Status: Fixed » Closed (fixed)

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