Issue 846 Add Breadcrumbs component #935

anprok · 2018-02-01T16:25:14Z

Make sure these boxes are checked before asking for review of your pull request - thank you!

General checks

All coding styles are fulfilled and there are no any issues reported by CodeSniffer CI.
All tests are running and there are no failed tests reported by CI.
Documentation has been updated according to PR changes.
Steps for review have been provided according to PR changes.
Make sure you've provided all necessary hook_update_N to support upgrade path.
Make sure your git email is associated with account on drupal.org, otherwise you won't get commits there.
If you would like to get credits on drupal.org, check documentation.

Thank you for your contribution!
#846

Steps for review

login as admin
open any page, for example programs/health-and-fitness/weight-loss/lose-win-weight-loss-program/class-times?location=1
see breadcrumb line under header menu
see in breadcrumb missing part without own page - programs , lose-win-weight-loss-program
switch to mobile - breadcrumb is missing

…profile

ddrozdik · 2018-02-05T12:16:10Z

I do not like the implementation via paragraph. In 90-95% all pages should have breadcrumbs enabled by default. Using paragraph requires adding it all the time manually. This is a lot of work if you need to create 1000 pages. We do not need a new paragraph in the list of paragraphs.
For example, YMCA of Greater TwinCities, YMCA of Long Island and YMCA of Metropolitan Dallas uses breadcrumbs almost on each page.

I think what we need here:

add easy_breadcrumb module
add a region to openy_roze to support breadcrumbs
add breadcrumbs block to the region
add styles to breadcrumbs. See examples:
add a fix for this issue Breadcrumbs component #846 (comment)
add Behat tests, to check breadcrumbs are available on the page
add documentation where to find breadcrumbs and how to configure in case you want to hide them from specific pages.

ddrozdik · 2018-02-06T14:00:28Z

composer.json

@@ -145,9 +145,6 @@
            "drupal/entity_embed": {
                "2511404 - Image entities/fields embedded using Entity Embed cannot be linked in CKEditor": "https://www.drupal.org/files/issues/entity_embed_links-2511404-31.patch"
            },
-            "drupal/datalayer": {


why was this patch removed?

This patch already added to the latest version of datalayer module https://www.drupal.org/node/2857270

ddrozdik · 2018-02-06T14:01:59Z

drupal-org.make

@@ -67,6 +67,7 @@ projects[views_field_formatter] = 1.5
 projects[lndr] = 1.11
 projects[crop] = 1.3
 projects[focal_point] = 1.0-beta5
+projects[east_breadcrumb] = 1.6


incorrect name of the module.

ddrozdik · 2018-02-06T14:10:15Z

themes/openy_themes/openy_rose/templates/page/page.html.twig

@@ -126,7 +126,11 @@

    {{ page.header }}

-    {{ page.breadcrumb }}
+    <div class="breadcrumbs">


we should not print region wrappers in case there is no content inside. I would recommend you move these wrappers into block template. You can find an example in the classy theme from the core.

ddrozdik · 2018-02-06T14:11:29Z

Please update config to enable following settings by default:

gundevel · 2018-02-07T09:40:55Z

Build comment file:

Environment	Link
Fresh OpenY installation	http://ci.openymca.org/build280
Upgraded(upgrade path) installation	http://upgrade.openy-dev.ffwua.com/build280
Installation process	http://install.openy-dev.ffwua.com/build280/install.php

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.openymca.org:8080/job/PR_BUILDER_COMPOSER/280/

gundevel · 2018-02-07T09:41:27Z

Acessibility Sniffer: front page checking WCAG2AA http://ci.openymca.org/build280/frontWCAG2AAhtmlcs.txt
Acessibility Sniffer: join page checking WCAG2AA http://ci.openymca.org/build280/joinWCAG2AAhtmlcs.txt
Acessibility Sniffer: locations page checking WCAG2AA http://ci.openymca.org/build280/locationsWCAG2AAhtmlcs.txt
Acessibility Sniffer: schedules page checking WCAG2AA http://ci.openymca.org/build280/schedulesWCAG2AAhtmlcs.txt
Acessibility Sniffer: blog page checking WCAG2AA http://ci.openymca.org/build280/blogWCAG2AAhtmlcs.txt

gundevel · 2018-02-07T09:59:55Z

Behat test results file URL http://ci.openymca.org/build280/profiles/contrib/openy/build/reports/behat and failed screenshots http://ci.openymca.org/build280/profiles/contrib/openy/build/reports/behat/screenshots

anprok added 6 commits January 31, 2018 19:11

#846 - Add easy_breadcrumb and breadcrumbs paragraph to composer and …

86f7e73

…profile

#846 - Remove already merged patch for datalayer module

21d6c27

#846 - Update composer lock

bbea6d8

#846 Add breadcrumbs paragraph

33ac654

#846 Add style and template for breadcrumb paragraph

bf64ca6

#846 Updates to breadcrumbs paragraph config

e306a17

ymcatwincities deleted a comment from gundevel Feb 1, 2018

#846 Add breadcrumbs paragraph documentation

836e6e0

ymcatwincities deleted a comment from gundevel Feb 2, 2018

ddrozdik added Type: Feature Completely new functionality. PR: Needs Review Needs someone review ( code ) PR: Needs Testing Manual testing is needed labels Feb 2, 2018

ddrozdik added this to the Version 1.8 milestone Feb 2, 2018

ymcatwincities deleted a comment from gundevel Feb 5, 2018

ymcatwincities deleted a comment from anprok Feb 5, 2018

ddrozdik added PR: Needs Work Unfinished task. Issues still there and removed PR: Needs Review Needs someone review ( code ) labels Feb 5, 2018

anprok added 6 commits February 6, 2018 00:21

#846 Add breadcrumb region to the openy_rose theme

bf21b52

#846 Remove breadcrumbs paragraph

96d80b8

#846 Import breadcrumbs block to display it in breadrumbs region

3c41f77

#846 Update breadcrumb style

6d92ada

#846 Remove breadcrum paragraph template and docs

16cff45

#846 Update breadcrumb style

ca981f3

ymcatwincities deleted a comment from gundevel Feb 6, 2018

AlexNetman added PR: Tested Manually tested. Green light for merging and removed PR: Needs Testing Manual testing is needed labels Feb 6, 2018

anprok added 2 commits February 6, 2018 15:53

#846 Add behat test for breadcrumb feature

8c1acd6

#846 Fix typo

e181340

ymcatwincities deleted a comment from gundevel Feb 6, 2018

ddrozdik suggested changes Feb 6, 2018

View reviewed changes

anprok added 4 commits February 6, 2018 17:52

#846 Move wrapper to breadcrumb block template

25dab35

#846 Fix module name

bad7fd9

#846 Adjust easy_breadcrumb settings

35689b5

#846 Update behat test after new settings

fbecd05

ymcatwincities deleted a comment from gundevel Feb 7, 2018

ddrozdik approved these changes Feb 7, 2018

View reviewed changes

ddrozdik added PR: Code Reviewed Code reviewed. Ready for QA and removed PR: Needs Work Unfinished task. Issues still there labels Feb 7, 2018

ddrozdik merged commit eea6daf into ymcatwincities:8.x-1.x Feb 7, 2018

anprok deleted the issue-846 branch August 27, 2018 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 846 Add Breadcrumbs component #935

Issue 846 Add Breadcrumbs component #935

anprok commented Feb 1, 2018 •

edited by ddrozdik

ddrozdik commented Feb 5, 2018 •

edited

ddrozdik Feb 6, 2018

anprok Feb 6, 2018 •

edited

ddrozdik Feb 6, 2018

ddrozdik Feb 6, 2018

ddrozdik commented Feb 6, 2018

gundevel commented Feb 7, 2018

gundevel commented Feb 7, 2018

gundevel commented Feb 7, 2018

Issue 846 Add Breadcrumbs component #935

Issue 846 Add Breadcrumbs component #935

Conversation

anprok commented Feb 1, 2018 • edited by ddrozdik

General checks

Steps for review

ddrozdik commented Feb 5, 2018 • edited

ddrozdik Feb 6, 2018

Choose a reason for hiding this comment

anprok Feb 6, 2018 • edited

Choose a reason for hiding this comment

ddrozdik Feb 6, 2018

Choose a reason for hiding this comment

ddrozdik Feb 6, 2018

Choose a reason for hiding this comment

ddrozdik commented Feb 6, 2018

gundevel commented Feb 7, 2018

Build comment file:

gundevel commented Feb 7, 2018

gundevel commented Feb 7, 2018

anprok commented Feb 1, 2018 •

edited by ddrozdik

ddrozdik commented Feb 5, 2018 •

edited

anprok Feb 6, 2018 •

edited