Problem/Motivation

Enable shortcut.module_link setting for bartik to show shortcut action link. Either by importing it from admin/config/development/configuration/single/import
Configuration type: Simple configuration
Configuration name: bartik.settings
Paste your configuration here:

third_party_settings:
  shortcut:
    module_link: true

Or execute this code form devel/php

\Drupal::configFactory()->getEditable('bartik.settings')->set('third_party_settings.shortcut.module_link', TRUE)->save(TRUE);

Or run this form root directory.

drush ev "\Drupal::configFactory()->getEditable('bartik.settings')->set('third_party_settings.shortcut.module_link', TRUE)->save(TRUE);"

or uninstall batrik put bartik.settings.yml in core/themes/bartik/config/install with following code 

third_party_settings:
  shortcut:
    module_link: true

Install bartik and set as default.

Proposed resolution

Fixed the styling

Remaining tasks

Review.
Commit.

User interface changes

Before


After

LTR


RTL


API changes

None

Data model changes

None

CommentFileSizeAuthor
#10 fix_shortcut_action-2544390-7.patch830 bytesemma.maria
#9 Screen Shot 2015-09-11 at 16.43.09.png31.94 KBemma.maria
#9 Screen Shot 2015-09-11 at 16.42.55.png22.32 KBemma.maria
#7 Screen Shot 2015-09-11 at 12.59.50.png48.79 KBemma.maria
#7 Screen Shot 2015-09-11 at 12.59.43.png39.66 KBemma.maria
#7 interdiff-0-7.txt778 bytesemma.maria
fix-batrik-shortcut-action-link.patch482 bytesjibran
Screenshot from 2015-08-03 02:32:45.png7.45 KBjibran
screenshot-d8.dev 2015-08-03 02-32-33.png5.7 KBjibran
screenshot-d8.dev 2015-08-03 02-37-01.png6.05 KBjibran
Screenshot from 2015-08-03 02:37:13.png7.64 KBjibran
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented

Should we also be able to set the display of the shortcut in the theme settings? It didn't seem to work for me.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented

I followed the steps to enable Shortcut for Bartik and I couldn't get it to work.

Is it here, like this?

function shortcut_install() {
  // Theme settings are not configuration entities and cannot depend on modules
  // so to set a module-specific setting, we need to set it with logic.
  if (\Drupal::service('theme_handler')->themeExists('bartik')) {
    \Drupal::configFactory()->getEditable('bartik.settings')->set('third_party_settings.shortcut.module_link', TRUE)->save(TRUE);
  }
}

Nothing happens even on reinstall.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Issue summary: View changes

#1

Should we also be able to set the display of the shortcut in the theme settings? It didn't seem to work for me.

@LewisNyman yes it is possible in D7. In D8 it's a third party setting so you have to import it using import page.

third_party_settings:
  shortcut:
    module_link: true

@emma.maria Try this form root dir.

drush ev "\Drupal::configFactory()->getEditable('bartik.settings')->set('third_party_settings.shortcut.module_link', TRUE)->save(TRUE);"

\Drupal::service('theme_handler')->themeExists('bartik') only returns true if bartik is installed. Perhaps it was not firing for you because of that. OTOH you found a new bug in shortcut. if condition should have !\Drupal::service('config.installer')->isSyncing() in it like this.

/**
 * Implements hook_install().
 */
function shortcut_install() {
  // Theme settings are not configuration entities and cannot depend on modules
  // so to set a module-specific setting, we need to set it with logic.
  if (!\Drupal::service('config.installer')->isSyncing() && \Drupal::service('theme_handler')->themeExists('baritk')) {
    \Drupal::configFactory()->getEditable('bartik.settings')->set('third_party_settings.shortcut.module_link', TRUE)->save(TRUE);
  }
}

I have also updated the issue summary with possible ways to enable this setting. Thanks for having a look at it folks.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

I ended up creating two issues one for shortcut module one documentation issue #2550385: Update hook_install documentation for editing configuration & #2550357: Fix shortcut_install() and shortcut_themes_installed(): Only update config when syncing flag is FALSE. @emma.maria++

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented

Thanks @jibran for adding instructions for enabling this. Let's get this tested and fixed!

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue tags: +frontend, +CSS
emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue summary: View changes
FileSize
interdiff-0-7.txt778 bytes
Screen Shot 2015-09-11 at 12.59.43.png39.66 KB
Screen Shot 2015-09-11 at 12.59.50.png48.79 KB

Yay I managed to get Shortcut enabled in Bartik this time!

I took a look at the styles, I agree that the existing ones still need to exist and I also agree with the styles added in the patch.

I also added the correct file comment for CSS files in Core plus added a small visual change to space the star icon a little more away from the page title.

Here are the screenshots:
 

 
On hover
 

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

Yay I managed to get Shortcut enabled in Bartik this time!

PHEW
Looks perfect but we need an actual patch not just interdiff. :P And after RTL screenshot I think we are RTBC here.
Thanks @emma.maria for the fixes.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue summary: View changes
FileSize
Screen Shot 2015-09-11 at 16.42.55.png22.32 KB
Screen Shot 2015-09-11 at 16.43.09.png31.94 KB

Oops apologies about missing the patch.

Also RTL screenshots:
 

 

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
FileSize
fix_shortcut_action-2544390-7.patch830 bytes

I will get there eventually.... here's the patch.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Issue summary: View changes
Issue tags: +Quickfix

Finally patch made an entrances. :D

I think we are done here. Screenshots look good. Updated issue summary with new screenshots.

Thanks @emma.maria for the RTL screenshots.

shahzad-anwar’s picture

shahzad-anwar CreditAttribution: shahzad-anwar commented
Status: Needs review » Reviewed & tested by the community

I tried the patch and it fixes the issue and works for both LTR and RTL Layouts.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 9564809 and pushed to 8.0.x. Thanks!

  • alexpott committed 9564809 on 8.0.x 
    Issue #2544390 by emma.maria, jibran: Fix shortcut action link styling...

Status: Fixed » Closed (fixed)

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