Skip to content
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

Merged

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Jun 29, 2021

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

@GuySartorelli GuySartorelli changed the title FIX Get anchor links from elemental pages NEW Get anchor links from elemental pages May 25, 2022
@GuySartorelli GuySartorelli force-pushed the pulls/4/anchor-blocks branch 4 times, most recently from c6e326a to bd813b9 Compare May 25, 2022 05:44
@GuySartorelli GuySartorelli marked this pull request as draft May 25, 2022 05:49
@GuySartorelli
Copy link
Member

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.

@sabina-talipova
Copy link
Contributor

sabina-talipova commented Jun 2, 2022

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.

  1. I create pages using the following template.
<?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;
        }
    }
}
  1. In each ElementalArea I add a Content block.
  2. In the Content block I add content via Source code.
<p>
     <a id="my_link" name="my_link">My_link</a>
     Test
</p>
<p id="my_text">Tests2</p>
  1. Save and publish ElementalArea.
  2. In the Content Editor for the Summary, I click on "Insert Link" and select "Anchor on a page".
  3. In the popup window that opens, in the "Select a page" I select this page with which I am working at the moment.
  4. But then In the "Anchor" drop-down list, I see "No results found".

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.

@GuySartorelli
Copy link
Member

GuySartorelli commented Jun 2, 2022

@sabina-talipova Note in the PR functionality added to the ElementalPageExtension (i.e. updateAnchorsOnPage) - it is my expectation that people will be using that extension when adding elemental blocks to their pages. Perhaps this PR should also add some documentation that advises people who (for whatever reason) don't use that extension to also add this new functionality to their pages directly?

@sabina-talipova
Copy link
Contributor

sabina-talipova commented Jun 3, 2022

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?

@GuySartorelli
Copy link
Member

GuySartorelli commented Jun 6, 2022

@sabina-talipova
Can you please try following these steps and let me know if that works?

  1. Create a new page with elemental blocks
  2. On the new page, create a content elemental block
  3. Add some anchor link to the content elemental block.
  4. Save the page
  5. In that, or any other block (including on a different page), click "Insert Link", then "Anchor on a page" and select the above page.

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.

@sabina-talipova
Copy link
Contributor

sabina-talipova commented Jun 7, 2022

@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.

@sabina-talipova
Copy link
Contributor

sabina-talipova commented Jun 7, 2022

@GuySartorelli,
I fixed problem in my local environment. But I have a question about expected behavour.
I have a WISIWYG on the Page. And I have ElementalArea created by ElementalPageExtension, with Content Block(WISIWYG). Both WISIWYG blocks have the links with id attribute as anchor. And both WISIWYG have ability to add a link that created in Elemental Area, but don't have any links that exists in Page's WISIWYG in the dropdown list in "Link to an anchor on a page" modal.
Please, advise me what I should do.

@GuySartorelli
Copy link
Member

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.

@sabina-talipova
Copy link
Contributor

sabina-talipova commented Jun 8, 2022

@GuySartorelli ,

I've finished testing in my local environment. Nice work! All works perfectly fine even with changes that I merged for TinyMCE_sslink-anchor.js. BUT with a few tiny enchincement that should be done.

First of all, in SiteTree class in the method getAnchorsOnPage() would be nice to implement your solution for DBHTMLText field. Since currently in this method we are looking for $this->Content (https://github.com/silverstripe/silverstripe-cms/blob/77b5bfa497d5026ffc806f1071a6c5d04d65bfb4/code/Model/SiteTree.php#L3338). But if I have other field's name , for example Summary, it skips this field. That was the reason for my issue, I hadn't Content field at all. So if you implement your changes, something like this, it helps us to solve problem where we have more then one DBHTMLText on the page with different names.

    /**
     * @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.

@sabina-talipova
Copy link
Contributor

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 ([^\"']+). Actually, I would suggest to get rid of all second part of condition \s+(name|id)\s*=\s*([^\"']+)[\s +>].
I'll put comment in the code.

src/Models/BaseElement.php Show resolved Hide resolved
src/Models/BaseElement.php Show resolved Hide resolved
src/Models/BaseElement.php Show resolved Hide resolved
@GuySartorelli
Copy link
Member

in SiteTree class in the method getAnchorsOnPage() would be nice to implement your solution for DBHTMLText field.

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 Summary even without elemental installed.

Copy link
Contributor

@sabina-talipova sabina-talipova left a 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.

@sabina-talipova sabina-talipova merged commit a1911ae into silverstripe:4 Jun 13, 2022
@sabina-talipova sabina-talipova deleted the pulls/4/anchor-blocks branch June 13, 2022 22:02
menno-rdmkr pushed a commit to busting-bytes/silverstripe-elemental that referenced this pull request Dec 15, 2022
…nchor-blocks

All Works as expected in local environment. All related tasks were approved and merged. Tests passed. MERGED
@micschk
Copy link
Contributor

micschk commented Feb 15, 2023

Note the duplicated anchor-in-page regex seems to pick up the "id" part of image-shortcodes (and possibly other shortcodes) as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants