FIX for #1312 to ensure preview pain state is properly maintained when editing page URL segment.

[patricknelson]

Addresses #1312. Should help ensure state in the preview pane is properly maintained when user is NOT in draft mode (aka internally as "Stage").

[dhensby]

@patricknelson is this suffix being set properly in the right places?

[patricknelson]

You're right. I'm also realizing there's no nice clean, normalized and centralized way to properly maintain state just yet (per se). That is, there's a $('.cms-preview').entwine('.ss.preview').getCurrentStateName(); method but that only returns the name of a link tag and itself must be (likely hackily) converted from LiveLink for example to Live which is pretty crusty.

On top of that, the link that this is associated with (for the URL segment) once updated doesn't yet point to a valid URL. That is, if my URL segment is currently about-us and change it to our-company, it will link itself to our-company for some odd reason even though it's not yet live since I've not even saved the draft. Plus there's the HTML embedded version of the URL (the current correct one, honestly) which is contained in LiveLink and StageLink.

So I'd actually propose eliminating this completely, actually. That is, the use of the data- attributes because, why duplicate maintenance of state? So hacky. Instead, let's just LINK to the LiveLink and StageLink (still referencing about-us, continue to update the text to point to the prospective/desired URL our-company with a note below the URL Segment indicating that the user must click "Save Draft" before the URL itself actually is reflected. Then, I'd also suggest taking a moment to also think of how to display that link (since it's not an editable field) if it differs from the published version.

Thoughts?

[patricknelson]

Here's a rough comp of what I'm referring to. If you ever edit the field, it will show this first message. If you have a URL segment that differs from draft vs. published, the second message (or something similar) would have to be displayed. Visually the link is updated, but it would technically still point to either StageLink or LiveLink (with value in <a> tag somewhere in the DOM) until after you save the page and the link you click is the link you get, otherwise (as long as you haven't modified the link).

[tractorcow]

The suffix should include the ?stage= in it; See SiteTreeURLSegmentField::getURL(), which joins this value on as though it was a full querystring.

setURLSuffix is never set anywhere, so this change would currently break. You'll need to make sure this method defaults to ?stage=Stage if it's not set.

[patricknelson]

Noted. Please see the conversation here for a suggestion on divergence from this implementation: #1449

[tractorcow]

Huh where? This thread? Sorry I didn't get that from the conversation above... I guess I'm just slow today. :D

[sminnee]

I think it makes sense to show a warning message when there is an unsaved or unpublished URL change.

I think that the visual representation you've mocked up is a bit on the large side; I'm not sure if @clarkepaul has ideas for how we could indicate to the user that the URL has been:

• changed but not yet saved
• changed and saved on draft but not yet published

I also wonder why URL would be the only field where we indicated at a field-specific level that there are unsaved changes.

[patricknelson]

Right @sminnee think of the mock-up more as a wireframe than an actual mock-up, then. It's too easy to get caught up in that, but more important is the concept and verbiage to hash out first rather than appearances. I do agree that it's pretty big! Maybe a bit too much for sure.

Also re:

I also wonder why URL would be the only field where we indicated at a field-specific level that there are unsaved changes.

It's because this is the only field that links directly to the front-end, which is confusing because the UI there (the <a> anchor) is consistent with what's currently in draft mode, or even unsaved, but it's pointing to the front-end where those unsaved (draft) or unpublished changes possibly aren't yet available.

Side note: Then there's the preview mode... this disambiguation helps to resolve that. Should also take into account current preview mode (i.e. Stage vs Live) which is a state that I think is mostly maintained in JavaScript as well (see LeftAndMain.Preview.js).

Also, to make sure we're covering all scenarios properly, maybe it's best to think through each situation. The variables are the page status (published vs. unpublished) and changes (saved or unsaved). Spelled out, that creates this list of 6 possible current states:

#	Page Status	Changes Saved
1.	Published	Saved
2.	Published	Unsaved
3.	Published w/ draft	Saved
4.	Published w/ draft	Unsaved
5.	Draft only	Saved
6.	Draft only	Unsaved

So we should walk through each one and say "Does this solution fit this scenario? Yes/No" and then I think we can move forward. For example, looking at state 6 tells me already there's a third thing we would need to do: Maybe a little flag/warning indicating that this URL will not be available for preview until after you save it. Also thinking about what the user see is their current preview mode is Live and they click to view a link that's only visible in draft only. Does that change the current preview mode to draft if I click that and it opens in a new window (probably not)? Then does the user get a 404? Does the user see it still, but specifying ?Stage=Stage in the URL doesn't change global state (good thing since then you'd have to worry about toggling the preview pane as well, not worth it).

[patricknelson]

Here's a further breakdown of what could happen in each scenario for your review.

IMPORTANT: This table assumes that the URL differs between published/draft! The notes below are not relevant, otherwise. Link 1 is the most recent link (either unsaved or current draft, if it differs from published) and link 2 is the link that would exist in any sort of warning message.

#	Page Status	Changes Saved	What to do?	Link 1	Link 2
1.	Published	Saved	Show one link	Open with current preview mode suffix.	NA
2.	Published	Unsaved	Two links, warn that page must be saved before URL becomes visible in either draft/published mode.	Current preview mode (new segment, likely 404)	Current preview mode
3.	Published w/ draft	Saved	Two links, warn that URL is draft only. Both links ignore preview mode setting	Link to Draft	Link to Published
4.	Published w/ draft	Unsaved	Two links, warn that page must be saved before URL becomes active in either draft/published mode.	Current preview mode (new segment, likely 404)	Current preview mode
5.	Draft only	Saved	One link, note that URL is only visible in draft mode.	Link to Draft	NA
6.	Draft only	Unsaved	Two links, warn that page must be saved before URL becomes visible in either draft/published mode.	Link to Draft (new segment, likely 404)	Link to Draft

Let me know if you want a plaintext version of the markdown for this table so you can tweak yourself without having to recreate it (tedious).

[tractorcow]

I'm still waiting for a response to my query... it was responded to with a link straight back to this same PR?

[patricknelson]

@tractorcow Could you provide more context to what you're saying here? Which query? What response?

[dhensby]

@patricknelson #1449 (comment)

[patricknelson]

Thanks @dhensby but I'm not sure that comment has any query per se, so I'm still confused. I don't think there's anything for me to do here as I believe the ball is in your court to help me decide on what you'd prefer to do in this situation. Please see my comments above (the big ones with the fancy tables) which I believe should be taken into account before we make any real changes per se.

Specifically #1449 (comment) and the one immediately after that. We should take a moment to think through all of the implications of how this field should link to the front-end page, since, in this case, one solution here may not fit all situations that would arise.

[tractorcow]

@patricknelson what I meant was, the code should look as below

'data-suffix' => $this->getURLSuffix(),

Not

'data-suffix' => '?stage=' . $this->getURLSuffix(),

Sorry if I wasn't making this clear.

[patricknelson]

Holy dooley @tractorcow! I see what you mean now, that makes more sense. Do you have the power (or patience) to read through and make a call on the outstanding question I had?

[tractorcow]

My rushed answer is that I don't think it would help to have a second link... it would just make things confusing.

If a link is changed but would result in a 404 if clicked (i.e. unsaved) then I could see adding a css visibility change (maybe make it red) with a small note. E.g. note: url not available until saved or something to that effect. That would be the limit of what I would do to address this issue.

Regarding this pull request; #1312 is addressed by this issue, but you now break the functionality, as there is no default assigment for this field. You could put a $this->setURLSuffix('?stage=Stage'); in the constructor to make this semver.