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

Merge SiteTreeURLSegmentField and SegmentField #2786

Open
5 tasks
maxime-rainville opened this issue Oct 7, 2022 · 5 comments
Open
5 tasks

Merge SiteTreeURLSegmentField and SegmentField #2786

maxime-rainville opened this issue Oct 7, 2022 · 5 comments

Comments

@maxime-rainville
Copy link
Contributor

maxime-rainville commented Oct 7, 2022

There's SiteTreeURLSegmentField in the CMS module and a derivative SegmentField in its own module. The derivative is used by UserForm.

SegmentField seems generally better in every way and is more flexible since it allows you to use it for any DataObject.

Acceptance criteria

  • The SegmentField module is folded back in to CMS or SegmentField becomes a dependency of CMS.
    • The SegmentField medule readme is updated accordingly
  • CMS start using SegmentField to manage URLSegment of SiteTrees
  • UserForm's usage of SegmentField is still functional and dependency on silverstripe/segment-field.
  • Current URLSegment filed UI is retained.

Notes

  • By itself, this is pretty low value, but it potentially saves us from having to make the SegmentField module CMS5 compatible.
  • Userform has not been made CMS5 compatible yet.
@GuySartorelli
Copy link
Member

NOTE: The segment field derivative seems more useful than just the way CMS uses it. It should probably be put in framework or admin, so projects which don't use this module can take advantage of it.

@maxime-rainville
Copy link
Contributor Author

If this looks like it's going to take more than a day drop it.

@emteknetnz emteknetnz self-assigned this Jan 17, 2023
@emteknetnz
Copy link
Member

emteknetnz commented Jan 17, 2023

I've spent some time getting SegmentField working on SiteTree and I just don't feel like this is worth doing considering the effort required, particularly when the motivation is "potentially saves us from having to make the SegmentField module CMS5 compatible." which is trivial

There's nothing "bad" about the existing functionality and I see little value consolidating this to only use SegmentField.

Out of the box this does not seem like a suitable substitute for SiteTreeURLSegmentField which has a bunch of custom functionality such as a hyperlink, automatically gets the domain prefixed + ?stage=Stage suffixed. We'd need to do extra work on SegmentField to make it behave like the existing SiteTreeURLSegmentField. This seems like a waste of effort when we already have working code.

I used the following code in case anyone wants to replicate

SiteTree::getCMSFields():2086

        $defaultUrlSegment = $this->generateURLSegment(_t(
            'SilverStripe\\CMS\\Controllers\\CMSMain.NEWPAGE',
            'New {pagetype}',
            ['pagetype' => $this->i18n_singular_name()]
        ));
        $urlsegment = SegmentField::create('URLSegment')
            ->setModifiers([
                SlugSegmentFieldModifier::create()->setDefault($defaultUrlSegment),
                ['-', ''],
                IDSegmentFieldModifier::create(),
            ])
            ->setPreview($baseLink . $this->URLSegment . '?stage=Stage')
            ->addExtraClass(($this->isHomePage() ? 'homepage-warning' : ''))

Page load - no hyperlnk
image

Click edit
image

Modify it - note js changetracker correctly picks up change and CMS "Save" and "Publish" buttons change to solid green
image

Click inline save - mistakently has -2 suffixed to it, also has lost the domain prefix and ?stage=Stage suffix
image

Click page save button
image

@emteknetnz
Copy link
Member

If this looks like it's going to take more than a day drop it.

Will close issue without doing work

@maxime-rainville maxime-rainville changed the title Merge SiteTreeURLSegmentField and SegmentFiled in CMS5 Merge SiteTreeURLSegmentField and SegmentFiled Jan 18, 2023
@maxime-rainville
Copy link
Contributor Author

I would still like this to happen in some way even if it's not in CMS5.

@GuySartorelli GuySartorelli changed the title Merge SiteTreeURLSegmentField and SegmentFiled Merge SiteTreeURLSegmentField and SegmentField Jan 18, 2023
@emteknetnz emteknetnz removed their assignment Jan 18, 2023
@GuySartorelli GuySartorelli added this to the Silverstripe CMS 6 milestone Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants