Problem/Motivation

D7 toolbar has tooltips on its links. D8 does not. This applies to top level links and deep links as well.

Proposed resolution

To be determined.

Remaining tasks

To be determined.

User interface changes

To be determined.

Report by moshe weitzman

Meta issue: #1846970: [meta] Responsive Toolbar follow-ups

CommentFileSizeAuthor
#20 1853324-missing-link-desc-11.patch2.27 KBRyanPrice
#11 1853324-missing-link-desc-11-test-only.patch1.83 KBkim.pepper
#11 1853324-missing-link-desc-11.patch2.28 KBkim.pepper
#3 missing-link-desc-1853324-3.png11.41 KBsandipmkhairnar
#3 missing-link-desc-1853324-3.patch457 bytessandipmkhairnar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Shyamala’s picture

Shyamala CreditAttribution: Shyamala commented
Issue tags: +toolbar-followup

Adding tags toolbar-followup

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Issue tags: +Novice

Adding tag novice

sandipmkhairnar’s picture

sandipmkhairnar CreditAttribution: sandipmkhairnar commented
FileSize
missing-link-desc-1853324-3.patch457 bytes
missing-link-desc-1853324-3.png11.41 KB

Added missing link description.

Missing link toolbar

sandipmkhairnar’s picture

sandipmkhairnar CreditAttribution: sandipmkhairnar commented
Status: Active » Needs review
cgalli’s picture

cgalli CreditAttribution: cgalli commented
Status: Needs review » Reviewed & tested by the community

manually tested, looks good
automated test missing

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented

Tested the patch and works as expected. The links appears in the toolbar.

Syntax seems ok. Just RTBC.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I have reviewed and applied the patch, it's working fine for me.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
CreditAttribution: klonos commented
Status: Reviewed & tested by the community » Active
Issue tags: +Needs tests

...as per #5. Other than that, RTBC from me too.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented

Adding novice-html, novice-php tags.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Issue tags: +html-novice, +php-novice

Oops, got the order of the terms backwards. Should have been html-novice, php-novice to match js-novice and css-novice.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented
Status: Active » Needs review
FileSize
1853324-missing-link-desc-11.patch2.28 KB
1853324-missing-link-desc-11-test-only.patch1.83 KB

Added test.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented

Hmm. Test-only fails locally.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented

#11: 1853324-missing-link-desc-11-test-only.patch queued for re-testing.

trevjs’s picture

trevjs CreditAttribution: trevjs commented

Reviewed and tested.

trevjs’s picture

trevjs CreditAttribution: trevjs commented
Status: Needs review » Reviewed & tested by the community
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Reviewed & tested by the community » Needs work

If the tests-only patch does not fail, the the tests apparently need work.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented
Status: Needs work » Needs review
Issue tags: -Needs tests, -Novice, -toolbar-followup, -html-novice, -php-novice

#11: 1853324-missing-link-desc-11.patch queued for re-testing.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented
Issue tags: +Needs tests, +Novice, +toolbar-followup, +html-novice, +php-novice

#11: 1853324-missing-link-desc-11-test-only.patch queued for re-testing.

RyanPrice’s picture

RyanPrice CreditAttribution: RyanPrice commented
Assigned: Unassigned » RyanPrice

Working on reroll

RyanPrice’s picture

RyanPrice CreditAttribution: RyanPrice commented
FileSize
1853324-missing-link-desc-11.patch2.27 KB

rerolled patch

RyanPrice’s picture

RyanPrice CreditAttribution: RyanPrice commented
Assigned: RyanPrice » Unassigned
rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Needs review » Reviewed & tested by the community

Patch applies well and everything seems to work well (we have links description again). Also the test looks good. Maybe RTBC?

alexpott’s picture

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

Committed 0e0a407 and pushed to 8.x. Thanks!

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

filing out the issue summary details under different headings and linking to meta issue