-
Notifications
You must be signed in to change notification settings - Fork 12
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
OEL-753: Integrate inpage navigation #137
Conversation
f3a692a
to
e340ecf
Compare
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
templates/patterns/inpage_navigation/pattern-inpage-navigation.html.twig
Outdated
Show resolved
Hide resolved
4cff2b6
to
b1d2eaf
Compare
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
06d993f
to
0a9ba71
Compare
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.
Please also check drone before you send this to review!
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
templates/patterns/inpage_navigation/pattern-inpage-navigation.html.twig
Outdated
Show resolved
Hide resolved
tests/src/Kernel/fixtures/markup_rendering_patterns/inpage_navigation.yml
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
templates/paragraphs/paragraph--oe-content-row--variant-inpage-navigation.html.twig
Outdated
Show resolved
Hide resolved
templates/patterns/inpage_navigation/inpage_navigation.ui_patterns.yml
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Show resolved
Hide resolved
$links_label = $sub_paragraph->get('field_oe_text')->first()->value; | ||
} | ||
elseif ($sub_paragraph->hasField('field_oe_plain_text_long') && !$sub_paragraph->get('field_oe_plain_text_long')->isEmpty()) { | ||
$links_label = $sub_paragraph->get('field_oe_plain_text_long')->first()->value; |
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.
Normally you should check first since you wrote the code, I did and indeed it can be way to long, this is a bad idea, we should use just field_oe_title
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
tests/src/Kernel/fixtures/markup_rendering_patterns/inpage_navigation.yml
Outdated
Show resolved
Hide resolved
modules/oe_bootstrap_theme_paragraphs/oe_bootstrap_theme_paragraphs.module
Outdated
Show resolved
Hide resolved
templates/paragraphs/paragraph--oe-content-row--variant-inpage-navigation.html.twig
Outdated
Show resolved
Hide resolved
templates/patterns/inpage_navigation/pattern-inpage-navigation.html.twig
Outdated
Show resolved
Hide resolved
tests/src/Kernel/fixtures/markup_rendering_patterns/inpage_navigation.yml
Outdated
Show resolved
Hide resolved
tests/src/Kernel/fixtures/markup_rendering_patterns/inpage_navigation.yml
Outdated
Show resolved
Hide resolved
16c9be3
to
840b382
Compare
…dd tiwg for title of oe_rich_text.
…bootstrap_theme_paragraphs.module.
…npageNavigationTest.
…t of types at testing.
…ation management.
840b382
to
a88dc53
Compare
@@ -0,0 +1,27 @@ | |||
inpage_navigation: | |||
label: "Inpage navigation" |
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.
Use single quotes for this and below
type: "render" | ||
label: "Content" | ||
description: "Content part of the inpage navigation." | ||
preview: "<h2 id='item-1'>Heading 1</h2><p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec ut imperdiet neque, finibus pretium metus. Mauris sed consequat purus, eget facilisis arcu. Vestibulum accumsan turpis sed dui placerat, in varius turpis consequat. Pellentesque porta id lorem sit amet efficitur. Sed id ipsum quis metus venenatis facilisis sit amet at mi.</p><h2 id='item-2'>Heading 2 with a long title going on several lines</h2><p>Ut placerat tellus dui, ut tempor orci pulvinar eget. Maecenas hendrerit risus vel tempus tincidunt. Suspendisse vitae ante nibh. Ut faucibus porttitor dui, sit amet pharetra nulla aliquam non. Donec massa mauris, convallis quis nunc nec, ultrices dapibus urna. In vel elementum ipsum, luctus volutpat odio. Maecenas aliquet justo in purus placerat dictum.</p><h2 id='item-3'>Heading 3</h2><p>Curabitur vehicula accumsan placerat. Aliquam eget tincidunt ex, sed elementum justo. Sed faucibus ultrices finibus. Vivamus pellentesque nisl ac interdum ultrices. Quisque ornare sit amet est sed pretium. Aliquam eu leo vitae nisi egestas volutpat ut non lectus. Sed condimentum tempus semper.</p><h2 id='item-4'>Heading 4</h2><p>Nullam dignissim quam tellus, eget pretium tortor vulputate quis. In vehicula enim vitae lorem fermentum convallis. Phasellus bibendum, turpis sit amet faucibus interdum, dui lorem iaculis enim, ac eleifend odio tellus eget dolor. Maecenas nunc ante, tincidunt nec risus ut, eleifend tempor nisi.</p>" |
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.
Single quotes please.
'#type': pattern | ||
'#id': inpage_navigation | ||
'#fields': | ||
title: "Page content" |
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.
Single quotes
'title': title, | ||
'links': links, | ||
'content': content, | ||
'id': 'bcl-inpage-navigation', |
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.
Is not needed because component hardcoded the id
description: 'Inpage navigation adds a navigation element and content element so when scrolling the active element is indicated in the nav part.' | ||
fields: | ||
title: | ||
type: 'text' |
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.
In 799 I'm leaving types without quotes because of reading on IDE, the type has a different color and improve my reading. This is a convention from myself and can be discussed if you want.
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.
Discussed with Francesco, he is agree doing in this way.
Jira issue(s):
(!) Don't merge until OEL-824 is merged at 1.x.