From code review at #3111409: Add new Olivero frontend theme to Drupal 9.1 core as beta

scripts.es6.js has this comment: // @todo, I'm not sure we even need the .mobile-buttons container anymore. - can that be resolved / removed?

CommentFileSizeAuthor
#4 3174088-applied-patch.png9.49 KBAbhijith S
#3 3174088-3.patch529 bytesRamya Balasubramanian
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mherchel created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Title: Remove .mobile-buttons comment within scripts.es6.js » Remove .mobile-buttons comment within Olivero's scripts.es6.js
Project: Olivero » Drupal core
Version: 8.x-1.x-dev » 9.1.x-dev
Component: Code » Olivero theme
Ramya Balasubramanian’s picture

Ramya Balasubramanian CreditAttribution: Ramya Balasubramanian at Srijan | A Material+ Company for Drupal India Association commented
Status: Active » Needs review
FileSize
3174088-3.patch529 bytes

Hi @mherchel,
Uploaded a patch. Please have a look.

Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
FileSize
3174088-applied-patch.png9.49 KB

Applied patch #3 and it is working fine.The .mobile-buttons comment within scripts.es6.js is removed after patch.

after

Abhijith S’s picture

Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs subsystem maintainer review

Would be great if one of the subsystem maintainers could confirm if this could be removed.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs review » Reviewed & tested by the community

Confirmed! That comment needs to be removed (it snuck over from the proof of concept html).

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Issue tags: -Needs subsystem maintainer review

Tag added in #6 addressed by #7.

  • lauriii committed 527fbc4 on 9.2.x 
    Issue #3174088 by Ramya Balasubramanian, mherchel: Remove .mobile-...

  • lauriii committed 2a6d4ac on 9.1.x 
    Issue #3174088 by Ramya Balasubramanian, mherchel: Remove .mobile-...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Reviewed & tested by the community » Fixed

Committed 527fbc4 and pushed to 9.2.x. Also cherry-picked to 9.1.x. Thanks!

Status: Fixed » Closed (fixed)

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