-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SHS-5905: Rework social media footer (backend) #1662
base: 11.5.1-release
Are you sure you want to change the base?
Conversation
…e profile works on tugboat
@cienvaras - This is ready for testing. I was able to solve the issue with the contextual menus. I see there is one test that is failing (hs_sandbox), but the rest pass so I think this one site is having a one-off problem? I re-ran the jobs a few times to see if it would clear. |
@codechefmarc I merged PR #1677 into the 11.5.1-release to fix the field_paragraph_style permissions. You can remove them from there now 👍 |
…-5905--social-block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codechefmarc In general everything works as expected, however the contextual menu access is still not working as expected. The contextual menu for the social media footer block is visible now, but other menus that were previously hidden (paragraphs for example) are visible too.
I also found a couple of things that might need attention:
- In some cases, when I click on the "Add another item" button the form scrolls to the bottom and the "Attributes" fieldset is focused. This is only happening to me on the initial block creation, not on other edits.
- The previous implementation allowed
mailto:
links. buturl
field validation doesn't allow them. This was in the original requirements but I checked the existing sites and only one site is using this kind of links. I'll check with Albert if this is still a requirement.
Also, I updated the template and added the styles and logic for the icon selection. Let me know if you have any questions.
@@ -76,9 +76,7 @@ permissions: | |||
- 'administer main menu items' | |||
- 'administer nodes' | |||
- 'administer users' | |||
- 'assign author role' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're changing here a lot of permission unrelated to the block itself, please update to include only the necessary permissions.
'link_title' => [ | ||
'#type' => 'textfield', | ||
'#title' => $this->t('Label'), | ||
'#description' => $this->t('If empty the domain name will be used.'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This help text in the specs was inaccurate, please change it to "If empty, the social platform name will be used for popular platforms. If the platform is unknown then the domain name will be used."
!\Drupal::currentUser()->hasPermission('view all contextual links') | ||
!in_array($group, ['media', 'block_content', 'hs_blocks.social_media_block']) && | ||
!$current_user->hasPermission('view all contextual links') && | ||
!$current_user->hasPermission('edit social media block') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is allowing any user with the "edit social media block" to see any contexual menu, including types that were previously exclude like paragraphs. It's necessary to ensure that only the block types in the list have contextual links for any user without the "view all contextual links" permission.
READY FOR REVIEW
Summary
Need Review By (Date)
2024-10-23
Urgency
low
Steps to Test
/admin/structure/block
PR Checklist