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

Fix "HIX score outdated" #2401

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Fix "HIX score outdated" #2401

merged 1 commit into from
Oct 31, 2023

Conversation

PeterNerlich
Copy link
Collaborator

Short description

Any action within the tinymce editor modifies the content, and should thus set the content to be dirty. At least, this is the assumption of tinymce. But there are actions that do not modify the content, like making a selection, or end up at a previous state, like adding a character back after accidentally removing it.

This PR fixes this problem by simply monkeypatching tinymce's isDirty() and setDirty() methods, always comparing it to a previous value and saving that on successful autosaves.
Evaluating the HIX score also sets such a value to use as basis for determining whether an earlier score is actually outdated or not.

Proposed changes

  • Monkeypatch tinymce isDirty() and setDirty() to actually represent the content being different from the last saved state
    • tinymce provides this nice startContent property that is initialised to the initial content and then never used anywhere
    • We compare against that and set it after successful autosaves (using the actual content that was sent when the autosave was triggered)
  • Have a similar value to compare whether the content is different from the last HIX evaluation and display the hints based on that

Side effects

  • isDirty() behaves differently now (not relying on the isNotDirty property anymore) and setDirty() tries to keep isNotDirty in line with what the new isDirty() would probably return if no changes to this are missed (this might get out of sync when an autosave is performed)

Additional notes

  • Where should I best put the monkeypatch?
  • Do we want a monkeypatch at all? (It seems very logical to me in this case, but it still feels… dirty 🙃)

Resolved issues

Fixes: #2300


Pull Request Review Guidelines

@PeterNerlich PeterNerlich requested a review from a team as a code owner August 17, 2023 08:30
@codeclimate
Copy link

codeclimate bot commented Aug 17, 2023

Code Climate has analyzed commit 1b137d5 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 76.7%.

View more on Code Climate.

@PeterNerlich PeterNerlich changed the title Fix outdated HIX score Fix "HIX score outdated" Aug 18, 2023
@PeterNerlich PeterNerlich force-pushed the bugfix/outdated_hix_value branch 3 times, most recently from e921225 to 316d4aa Compare August 18, 2023 15:22
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you for the PR 💪 It fixes the problem 🙂

But some side effect was found:

  1. an unsaved warning will be shown when some changes are really made and the page is published or saved as draft. For example, add some words in a page and save it.
  2. After leaving this unsaved warning by chosing "Verlassen" and saving the page several times, IntegrityError will be raised.

@PeterNerlich
Copy link
Collaborator Author

I cut out everything not related to this issue and put it in #2476

@MizukiTemma MizukiTemma force-pushed the bugfix/outdated_hix_value branch from 44fec1f to 1b137d5 Compare October 28, 2023 14:10
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Thank you for the updates 🙂 Looks good!

Copy link
Contributor

@seluianova seluianova left a comment

Choose a reason for hiding this comment

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

LGTM!

@PeterNerlich PeterNerlich merged commit 4370601 into develop Oct 31, 2023
@PeterNerlich PeterNerlich deleted the bugfix/outdated_hix_value branch October 31, 2023 16:00
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.

Copying text outdates the HIX-value
3 participants