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

[4.x] Prevent dirt on saved entry #9203

Merged
merged 2 commits into from
Dec 14, 2023
Merged

[4.x] Prevent dirt on saved entry #9203

merged 2 commits into from
Dec 14, 2023

Conversation

vluijkx
Copy link
Contributor

@vluijkx vluijkx commented Dec 14, 2023

In version 4.35.0, PR #8961 was introduced to show changes from the EntrySaving event in the edit-form UI.

However, the values from the response are in many edge-cases slightly different than the current values. This causes the form to become dirty when an entry is saved. This dirtiness is especially inconvenient when using revisions as it causes the publish button to become disabled. Issue #9068 is an example of this.

This PR disables dirty-tracking while resetting the values from the response.

I am new to the Statamic-codebase and do not know if a timeout is the best approach here. Setting the timeout to 300 did not work which is why I increased it to 350.

@ryanmitchell
Copy link
Contributor

You could use this.$nextTick instead of the timeout - you can see its being used for the emit event

@vluijkx
Copy link
Contributor Author

vluijkx commented Dec 14, 2023

Yeah, I have tried to do so like this:

this.trackDirtyState = false

this.values = this.resetValuesFromResponse(response.data.data.values,)

this.$nextTick(() => {
    this.$emit('saved', response)
    this.trackDirtyState = true
})

However, this does not prevent this.values = ... from triggering this.$dirt.add(...).

Copy link
Member

@duncanmcclean duncanmcclean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I've managed to get it working with $nextTick instead of using setTimeout.

@duncanmcclean duncanmcclean merged commit 48cc920 into statamic:4.x Dec 14, 2023
17 checks passed
@vluijkx
Copy link
Contributor Author

vluijkx commented Dec 15, 2023

@duncanmcclean Something went wrong here 😅.

CleanShot 2023-12-15 at 11 23 41

Is there a reason to use this.nextTick instead of this.$nextTick?

@duncanmcclean
Copy link
Member

Doh! No reason, I'll get that fixed up. Thanks for spotting!

@vluijkx
Copy link
Contributor Author

vluijkx commented Dec 15, 2023

Thanks a lot! 🙂

duncanmcclean added a commit that referenced this pull request Dec 15, 2023
Before merging #9203, I refactored it to use a `$nextTick` which I thought worked at the time but didn't.
duncanmcclean added a commit that referenced this pull request Dec 15, 2023
Revert to original dirty state fix

Before merging #9203, I refactored it to use a `$nextTick` which I thought worked at the time but didn't.
@duncanmcclean
Copy link
Member

@vluijkx Should be fixed in the next release. I ended up reverting to your original setTimeout fix because $nextTick didn't work when using Bard fields (#9213).

@vluijkx vluijkx deleted the fix/issue-9068 branch December 15, 2023 12:20
@Web10-Joris
Copy link

@duncanmcclean Just FYI. This is also preventing the 'Create & Link Item' action in the Entry Fieldtype from closing the newly created entry and linking it to the field. Downgraded back to 4.40.0 to check, then it worked again.

@duncanmcclean
Copy link
Member

@duncanmcclean Just FYI. This is also preventing the 'Create & Link Item' action in the Entry Fieldtype from closing the newly created entry and linking it to the field. Downgraded back to 4.40.0 to check, then it worked again.

Can you try updating to v4.42.0?

@Web10-Joris
Copy link

@duncanmcclean Yep thanks, it is working again in the new release!

@morhi
Copy link
Contributor

morhi commented Dec 21, 2023

Hey @duncanmcclean I think your change might have broken the Link fieldytpe. For instance, when you select "URL" in a Link, that placed anywhere within a Replicator set, it will reset, as soon as you start typing or change anything else in the content. I am using Statamic 4.42.0. Seems like the bug got introduced by 4.41.0, because downgrading to 4.40.0 fixes the issue. Any idea?

@duncanmcclean
Copy link
Member

That sounds like a different bug. Can you open a new issue and we can take a look in the new year?

@morhi
Copy link
Contributor

morhi commented Dec 21, 2023

That sounds like a different bug. Can you open a new issue and we can take a look in the new year?

Yes, I did that. That is a good idea. Get some rest and have a blessed Christmas time 🎄

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.

5 participants