Improve diffing behavior for text fields

CHANGELOG.txt

@@ -6,6 +6,7 @@ Changelog
 
  * Removed leftover Python 2.x compatibility code (Sergey Fedoseev)
  * Combine flake8 configurations (Sergey Fedoseev)
+ * Improved diffing behavior for text fields (Aliosha Padovani)
  * Fix: Rename documents listing column 'uploaded' to 'created' (LB (Ben Johnston))
  * Fix: Submenu items longer then the page height are no longer broken by the submenu footer (Igor van Spengen)
 

CONTRIBUTORS.rst

@@ -419,6 +419,7 @@ Contributors
 * Thijs Baaijen
 * Igor van Spengen
 * Stefani Castellanos
+* Aliosha Padovani
 
 Translators
 ===========

docs/releases/2.8.rst

@@ -14,16 +14,17 @@ What's new
 Other features
 ~~~~~~~~~~~~~~
 
- * ...
+ * Removed leftover Python 2.x compatibility code (Sergey Fedoseev)
+ * Combine flake8 configurations (Sergey Fedoseev)
+ * Improved diffing behavior for text fields (Aliosha Padovani)
 
 
 Bug fixes
 ~~~~~~~~~
 
- * Removed leftover Python 2.x compatibility code (Sergey Fedoseev)
- * Combine flake8 configurations (Sergey Fedoseev)
  * Rename documents listing column 'uploaded' to 'created' (LB (Ben Johnston))
  * Submenu items longer then the page height are no longer broken by the submenu footer (Igor van Spengen)
 
+
 Upgrade considerations
 ======================

wagtail/admin/compare.py

@@ -230,7 +230,14 @@ class ChoiceFieldComparison(FieldComparison):
         val_b = force_str(dict(self.field.flatchoices).get(self.val_b, self.val_b), strings_only=True)
 
         if self.val_a != self.val_b:
-            return TextDiff([('deletion', val_a), ('addition', val_b)]).to_html()
+            diffs = []
+
+            if val_a:
+                diffs += [('deletion', val_a)]
+            if val_b:
+                diffs += [('addition', val_b)]
+
+            return TextDiff(diffs).to_html()
         else:
             return escape(val_a)
 
@@ -593,7 +600,7 @@ def diff_text(a, b):
         tokens = []
         current_token = ""
 
-        for c in text:
+        for c in text or "":
             if c.isalnum():
                 current_token += c
             else:

wagtail/admin/edit_handlers.py

@@ -3,6 +3,7 @@ import re
 
 from django import forms
 from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured
+from django.db.models.fields import CharField, TextField
 from django.forms.formsets import DELETION_FIELD_NAME, ORDERING_FIELD_NAME
 from django.forms.models import fields_for_model
 from django.template.loader import render_to_string
@@ -500,6 +501,10 @@ class FieldPanel(EditHandler):
 
             if isinstance(field, RichTextField):
                 return compare.RichTextFieldComparison
+
+            if isinstance(field, (CharField, TextField)):
+                return compare.TextFieldComparison
+
         except FieldDoesNotExist:
             pass
 

wagtail/admin/tests/test_compare.py

@@ -67,10 +67,45 @@ class TestTextFieldComparison(TestFieldComparison):
         self.assertIsInstance(comparison.htmldiff(), SafeString)
         self.assertTrue(comparison.has_changed())
 
+    def test_from_none_to_value_only_shows_addition(self):
+        comparison = self.comparison_class(
+            SimplePage._meta.get_field('content'),
+            SimplePage(content=None),
+            SimplePage(content="Added content")
+        )
+
+        self.assertEqual(comparison.htmldiff(), '<span class="addition">Added content</span>')
+        self.assertIsInstance(comparison.htmldiff(), SafeString)
+        self.assertTrue(comparison.has_changed())
+
+    def test_from_value_to_none_only_shows_deletion(self):
+        comparison = self.comparison_class(
+            SimplePage._meta.get_field('content'),
+            SimplePage(content="Removed content"),
+            SimplePage(content=None)
+        )
+
+        self.assertEqual(comparison.htmldiff(), '<span class="deletion">Removed content</span>')
+        self.assertIsInstance(comparison.htmldiff(), SafeString)
+        self.assertTrue(comparison.has_changed())
+
 
-class TestRichTextFieldComparison(TestTextFieldComparison):
+class TestRichTextFieldComparison(TestFieldComparison):
     comparison_class = compare.RichTextFieldComparison
 
+    # Only change from FieldComparison is the HTML diff is performed on words
+    # instead of the whole field value.
+    def test_has_changed(self):
+        comparison = self.comparison_class(
+            SimplePage._meta.get_field('content'),
+            SimplePage(content="Original content"),
+            SimplePage(content="Modified content"),
+        )
+
+        self.assertEqual(comparison.htmldiff(), '<span class="deletion">Original</span><span class="addition">Modified</span> content')
+        self.assertIsInstance(comparison.htmldiff(), SafeString)
+        self.assertTrue(comparison.has_changed())
+
     # Only change from FieldComparison is that this comparison disregards HTML tags
     def test_has_changed_html(self):
         comparison = self.comparison_class(
@@ -329,6 +364,28 @@ class TestChoiceFieldComparison(TestCase):
         self.assertIsInstance(comparison.htmldiff(), SafeString)
         self.assertTrue(comparison.has_changed())
 
+    def test_from_none_to_value_only_shows_addition(self):
+        comparison = self.comparison_class(
+            EventPage._meta.get_field('audience'),
+            EventPage(audience=None),
+            EventPage(audience="private"),
+        )
+
+        self.assertEqual(comparison.htmldiff(), '<span class="addition">Private</span>')
+        self.assertIsInstance(comparison.htmldiff(), SafeString)
+        self.assertTrue(comparison.has_changed())
+
+    def test_from_value_to_none_only_shows_deletion(self):
+        comparison = self.comparison_class(
+            EventPage._meta.get_field('audience'),
+            EventPage(audience="public"),
+            EventPage(audience=None),
+        )
+
+        self.assertEqual(comparison.htmldiff(), '<span class="deletion">Public</span>')
+        self.assertIsInstance(comparison.htmldiff(), SafeString)
+        self.assertTrue(comparison.has_changed())
+
 
 class TestTagsFieldComparison(TestCase):
     comparison_class = compare.TagsFieldComparison