-
Notifications
You must be signed in to change notification settings - Fork 115
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
NEW Get anchor links from elemental pages #911
NEW Get anchor links from elemental pages #911
Conversation
1e5fb35
to
441aecb
Compare
c6e326a
to
bd813b9
Compare
I've taken over this PR. Marking this as draft so nobody merges it just yet - the functionality is all there but I still need to add test coverage. |
388152c
to
46bc83a
Compare
46bc83a
to
c1ffa4c
Compare
I'm testing these changes and I can't seem to get the result (the list of anchors on the page) in any way. Perhaps I'm doing something wrong. Here are the steps I follow.
<?php
namespace {
use DNADesign\Elemental\Models\ElementalArea;
use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
use SilverStripe\CMS\Model\SiteTree;
class Page extends SiteTree
{
private static $db = [
'Summary' => 'HTMLText',
];
private static $has_one = [
'BlocksMain' => ElementalArea::class,
'BlocksRight' => ElementalArea::class,
'BlocksFooter' => ElementalArea::class,
];
public function getCMSFields()
{
$fields = parent::getCMSFields();
$summary = HtmlEditorField::create('Summary', 'Summary', false);
$summary->setRows(5);
$summary->setDescription('Test Description');
$fields->addFieldsToTab('Root.Main',$summary, 'Summary');
return $fields;
}
}
}
<p>
<a id="my_link" name="my_link">My_link</a>
Test
</p>
<p id="my_text">Tests2</p>
I also tried adding an anchor, which is placed in the Summary, to the ElementalArea Content Block. And I even refreshed the page. Still don't have the anchors. |
@sabina-talipova Note in the PR functionality added to the |
I've applied extension to the Page, still nothing happens. class Page extends SiteTree
{
private static $extensions = [
ElementalPageExtension::class,
]; And also, if I add extension it creates ElementalArea by itself. But if I'll have two or more ElementalArea on a page, will your solution work with other ElementalAreas on a page or it works only with ElementalArea that was created by your Extension? |
Co-Authored-By: Steve Boyd <[email protected]>
@sabina-talipova
The anchor link you added should be in the anchors dropdown list. I suspect the issue you were having is that new anchors when you don't save the page aren't showing up - that's a defect with the way I implemented silverstripe/silverstripe-cms#2741 which I will need to fix - but in the meantime the main functionality here should work. |
@GuySartorelli , it still doesn't work and it looks like it doesn't invoke updateAnchorsOnPage method from ElementalPageExtension. ElementalPageExtension adds ElementalArea on the Page, but doesn't run updateAnchorsOnPage. Probably, I should make something else. |
@GuySartorelli, |
As discussed on slack, that sounds like a problem with the way I've changed the javascript in silverstripe/silverstripe-cms#2741, which means I'm not quite done yet. Once you've finished your review, assign this back to me and I'll see what I can do about that. |
I've finished testing in my local environment. Nice work! All works perfectly fine even with changes that I merged for First of all, in /**
* @return array
*/
public function getAnchorsOnPage()
{
$anchors = [];
$allFields = DataObject::getSchema()->fieldSpecs($this);
$anchorRegex = "/\\s+(?<attr>name|id)\\s*=\\s*([\"'])(?<token>[^\\2\\s>]*?)\\2|\\s+(name|id)\\s*=\\s*([^\"']+)[\\s +>]/im";
foreach ($allFields as $field => $fieldSpec) {
$fieldObj = $this->owner->dbObject($field);
if ($fieldObj instanceof DBHTMLText) {
$parseSuccess = preg_match_all($anchorRegex, $fieldObj->getValue() ?? '', $matches);
if ($parseSuccess >= 1) {
$fieldAnchors = array_values(array_filter(
array_merge($matches['token'])
));
$anchors = array_merge($anchors, $fieldAnchors);
}
}
}
$anchors = array_unique($anchors);
$this->extend('updateAnchorsOnPage', $anchors);
return $anchors;
} But it is just suggestion. |
The other changes that should be done, it is regular expression. I'm not sure that 5th group in regexp is looking for exactly what we need |
I won't make this change as a part of fixing the issue here. It could be a good enhancement so feel free to an issue for it in silverstripe/silverstripe-cms - but that is a separate problem to the issue this PR is fixing. That problem will exist for situations where people already have fields such as |
c1ffa4c
to
2ddc79c
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.
All works fine in local environment together with changes from PR silverstripe/silverstripe-cms#2742
Merge when all related PRs will be approved.
…nchor-blocks All Works as expected in local environment. All related tasks were approved and merged. Tests passed. MERGED
Note the duplicated anchor-in-page regex seems to pick up the "id" part of image-shortcodes (and possibly other shortcodes) as well. |
Adds anchor links in
HTMLText
fields on an elemental block to the "anchor on a page" link form in WYSIWYG fields.The new method should be overridden for blocks that need to add additional anchors or exclude some anchors from this base list.
An argument can be made that this should render the block and use that to parse out any anchors - but the approach this PR takes is more in line with how this is currently done in
SiteTree::getAnchorsOnPage()
.Also updated the
theHasAContentElementWithContent
fixture context regex to allow arbitrary html content in the test fixture.Dependencies
The following related PRs must be merged in first in order for tests to pass
Parent Issues