Merge pull request #35141 from dimagi/jls/update-bootstrap-docs

Updates to bootstrap docs and migration tool

corehq/apps/hqwebapp/management/commands/migrate_app_to_bootstrap5.py

@@ -1,14 +1,13 @@
 import time
-from pathlib import Path
 
-from django.conf import settings
 from django.core.management import BaseCommand
 
 from corehq.apps.hqwebapp.utils.bootstrap import BOOTSTRAP_3, BOOTSTRAP_5
 from corehq.apps.hqwebapp.utils.bootstrap.changes import (
     get_spec,
     make_direct_css_renames,
     make_numbered_css_renames,
+    make_select_form_control_renames,
     make_template_tag_renames,
     make_data_attribute_renames,
     make_javascript_dependency_renames,
@@ -17,7 +16,9 @@
     flag_stateful_button_changes_bootstrap5,
     flag_changed_javascript_plugins,
     flag_crispy_forms_in_template,
+    flag_file_inputs,
     flag_inline_styles,
+    flag_selects_without_form_control,
     add_todo_comments_for_flags,
     update_gruntfile,
 )
@@ -87,14 +88,6 @@ def add_arguments(self, parser):
         )
 
     def handle(self, app_name, **options):
-        if not settings.BOOTSTRAP_MIGRATION_LOGS_DIR:
-            self.stderr.write("\nPlease make sure BOOTSTRAP_MIGRATION_LOGS_DIR is "
-                              "set in your localsettings.py before continuing...\n\n")
-            self.stdout.write(self.style.MIGRATE_LABEL(
-                "TIP: path should be outside of this repository\n\n"
-            ))
-            return
-
         selected_filename = options.get('filename')
 
         is_app_migration_complete = is_app_completed(app_name)
@@ -328,7 +321,6 @@ def migrate_single_file(self, app_name, file_path, spec, is_template, review_cha
                 self.stdout.write(self.style.WARNING(
                     self.format_header(f"Finalizing changes for {short_path}...")
                 ))
-                self.record_file_changes(file_path, app_name, file_changelog, is_template)
                 if self.no_split:
                     self.migrate_file_in_place(app_name, file_path, new_lines, is_template)
                     if is_template:
@@ -403,31 +395,6 @@ def display_rename_summary(self, changelog):
         self.stdout.write("\n\n\nAnswering 'y' below will automatically make this change "
                           "in the Bootstrap 5 version of this file.\n\n")
 
-    def record_file_changes(self, template_path, app_name, changelog, is_template):
-        if is_split_path(template_path):
-            parent_dir = template_path.parent.parent
-        else:
-            parent_dir = template_path.parent
-        short_path = get_short_path(app_name, parent_dir, is_template)
-        readme_directory = Path(settings.BOOTSTRAP_MIGRATION_LOGS_DIR) / short_path
-        readme_directory.mkdir(parents=True, exist_ok=True)
-        extension = '.html' if is_template else '.js'
-        readme_filename = template_path.name.replace(extension, '.md')
-        readme_path = readme_directory / readme_filename
-        with open(readme_path, 'w') as readme_file:
-            readme_file.writelines(changelog)
-        self.show_information_about_readme(readme_path)
-
-    def show_information_about_readme(self, readme_path):
-        if self.no_split:
-            self.stdout.write("\nThe changelog summarizing the "
-                              "migration changes can be found here:\n")
-        else:
-            self.stdout.write("\nThe changelog for all changes to the Bootstrap 5 "
-                              "version of this file can be found here:\n")
-        self.stdout.write(f"\n{readme_path}\n\n")
-        self.stdout.write("** Please make a note of this for reviewing later.\n\n\n")
-
     def migrate_file_in_place(self, app_name, file_path, bootstrap5_lines, is_template):
         short_path = get_short_path(app_name, file_path, is_template)
         confirm = get_confirmation(f"Apply changes to '{short_path}'?", default='y')
@@ -612,6 +579,8 @@ def make_template_line_changes(old_line, spec, is_fresh_migration):
 
         new_line, attribute_renames = make_data_attribute_renames(new_line, spec)
         renames.extend(attribute_renames)
+        new_line, select_renames = make_select_form_control_renames(new_line, spec)
+        renames.extend(select_renames)
         new_line, template_dependency_renames = make_template_dependency_renames(new_line, spec)
         renames.extend(template_dependency_renames)
         new_line, template_tag_renames = make_template_tag_renames(new_line, spec)
@@ -623,7 +592,9 @@ def get_flags_in_template_line(template_line, spec):
         flags = flag_changed_css_classes(template_line, spec)
         flags.extend(flag_stateful_button_changes_bootstrap5(template_line))
         flags.extend(flag_crispy_forms_in_template(template_line))
+        flags.extend(flag_file_inputs(template_line))
         flags.extend(flag_inline_styles(template_line))
+        flags.extend(flag_selects_without_form_control(template_line))
         return flags
 
     @staticmethod

corehq/apps/hqwebapp/tests/utils/test_bootstrap_changes.py

@@ -3,6 +3,7 @@
     get_spec,
     make_direct_css_renames,
     make_numbered_css_renames,
+    make_select_form_control_renames,
     make_template_tag_renames,
     make_data_attribute_renames,
     make_javascript_dependency_renames,
@@ -13,7 +14,9 @@
     replace_path_references,
     check_bootstrap3_references_in_template,
     flag_crispy_forms_in_template,
+    flag_selects_without_form_control,
     check_bootstrap3_references_in_javascript,
+    flag_file_inputs,
     flag_inline_styles,
     make_template_dependency_renames,
     add_todo_comments_for_flags,
@@ -49,6 +52,15 @@ def test_make_numbered_css_renames_bootstrap5():
     eq(renames, ['renamed col-xs-<num> to col-sm-<num>'])
 
 
+def test_make_select_form_control_renames_bootstrap5():
+    line = """        <select data-bind:"visible: showMe" class="form-control">\n"""
+    final_line, renames = make_select_form_control_renames(
+        line, get_spec('bootstrap_3_to_5')
+    )
+    eq(final_line, """        <select data-bind:"visible: showMe" class="form-select">\n""")
+    eq(renames, ['renamed form-control to form-select'])
+
+
 def test_make_template_tag_renames_bootstrap5():
     line = """        {% requirejs_main "data_dictionary/js/data_dictionary" %}\n"""
     final_line, renames = make_template_tag_renames(
@@ -91,7 +103,7 @@ def test_flag_changed_css_classes_bootstrap5():
         line, get_spec('bootstrap_3_to_5')
     )
     eq(flags, [[
-        'css:dl-horizontal',
+        'css-dl-horizontal',
         '`dl-horizontal` has been dropped.\nInstead, use `.row` on `<dl>` and use grid column classes '
         '(or mixins) on its `<dt>` and `<dd>` children.\n\nAn EXAMPLE for how to apply this change is '
         'provided below.\nPlease see docs for further details.\n\nPreviously:\n```\n<dl class='
@@ -210,18 +222,26 @@ def test_check_bootstrap3_references_in_javascript():
     eq(issues, ['This javascript file references a bootstrap 3 file.'])
 
 
+def test_flag_file_inputs():
+    line = """<input type="file" id="xform_file_input" name="xform" />"""
+    flags = flag_file_inputs(line)
+    eq(len(flags), 1)
+    eq(flags[0][0], "css-file-inputs")
+    eq(flags[0][1].startswith('Please add the form-control class'), True)
+
+
 def test_flag_inline_styles():
     line = """method="post" style="float: left; margin-right: 5px;">"""
     flags = flag_inline_styles(line)
     eq(len(flags), 1)
-    eq(flags[0][0], "inline style")
+    eq(flags[0][0], "inline-style")
     eq(flags[0][1].startswith('This template uses inline styles.'), True)
 
 
 def test_flag_crispy_forms_in_template():
     line = """    {% crispy form %}\n"""
     flags = flag_crispy_forms_in_template(line)
-    eq(flags[0][0], "check crispy")
+    eq(flags[0][0], "crispy")
     eq(flags[0][1].startswith("This template uses crispy forms."), True)
 
 
@@ -230,7 +250,7 @@ def test_flag_changed_javascript_plugins_bootstrap5():
     flags = flag_changed_javascript_plugins(
         line, get_spec('bootstrap_3_to_5')
     )
-    eq(flags, [["plugin:modal",
+    eq(flags, [["js-modal",
                 "The `modal` plugin has been restructured since the removal of jQuery.\n\nThere is now a new way "
                 "of triggering modal events and interacting with modals in javascript.\nFor instance, if we "
                 "wanted to hide a modal with id `#bugReport` before, we would now do the\nfollowing..."
@@ -247,7 +267,7 @@ def test_flag_extended_changed_javascript_plugins_bootstrap5():
     flags = flag_changed_javascript_plugins(
         line, get_spec('bootstrap_3_to_5')
     )
-    eq(flags, [['plugin:popover',
+    eq(flags, [['js-popover',
                 'The `popover` plugin has been restructured since the removal of jQuery.\n'
                 '\nThere is now a new way of triggering popover events and interacting '
                 'with popovers in javascript.\n\nPlease feel free to update this help text'
@@ -257,6 +277,13 @@ def test_flag_extended_changed_javascript_plugins_bootstrap5():
                 'components/popovers/\n']])
 
 
+def test_flag_selects_without_form_control_bootstrap5():
+    line = """    <select\n"""
+    flags = flag_selects_without_form_control(line)
+    eq(flags[0][0], "css-select-form-control")
+    eq(flags[0][1].startswith("Please replace `form-control` with `form-select`."), True)
+
+
 def test_file_contains_reference_to_path():
     filedata = """
     {# Our Libraries #}
@@ -356,14 +383,14 @@ def test_add_todo_comments_for_flags_template():
         line, get_spec('bootstrap_3_to_5')
     )
     line = add_todo_comments_for_flags(flags, line, is_template=True)
-    eq(line, """          <div class="form-inline nav"  {# todo B5: css:form-inline, css:nav #}\n""")
+    eq(line, """          <div class="form-inline nav"  {# todo B5: css-form-inline, css-nav #}\n""")
 
 
 def test_add_todo_comments_for_flags_template_replace():
-    line = """          {% crispy form %}  {# todo B5: css:form-inline, css:nav #}\n"""
+    line = """          {% crispy form %}  {# todo B5: css-form-inline, css-nav #}\n"""
     flags = flag_crispy_forms_in_template(line)
     line = add_todo_comments_for_flags(flags, line, is_template=True)
-    eq(line, """          {% crispy form %}  {# todo B5: check crispy #}\n""")
+    eq(line, """          {% crispy form %}  {# todo B5: crispy #}\n""")
 
 
 def test_add_todo_comments_for_flags_template_noop():
@@ -381,16 +408,16 @@ def test_add_todo_comments_for_flags_javascript():
         line, get_spec('bootstrap_3_to_5')
     )
     line = add_todo_comments_for_flags(flags, line, is_template=False)
-    eq(line, """            $modal.modal({  /* todo B5: plugin:modal */\n""")
+    eq(line, """            $modal.modal({  /* todo B5: js-modal */\n""")
 
 
 def test_add_todo_comments_for_flags_javascript_replace():
-    line = """            $popover.popover({  /* todo B5: plugin:modal */\n"""
+    line = """            $popover.popover({  /* todo B5: js-modal */\n"""
     flags = flag_changed_javascript_plugins(
         line, get_spec('bootstrap_3_to_5')
     )
     line = add_todo_comments_for_flags(flags, line, is_template=False)
-    eq(line, """            $popover.popover({  /* todo B5: plugin:popover */\n""")
+    eq(line, """            $popover.popover({  /* todo B5: js-popover */\n""")
 
 
 def test_add_todo_comments_for_flags_javascript_noop():

corehq/apps/hqwebapp/utils/bootstrap/changes.py

@@ -102,6 +102,18 @@ def make_numbered_css_renames(line, spec):
     )
 
 
+def make_select_form_control_renames(line, spec):
+    if "<select" in line:
+        if re.search(_get_direct_css_regex("form-control"), line):
+            return _do_rename(
+                line,
+                {"form-control": "form-select"},
+                _get_direct_css_regex,
+                lambda x: r"\1form-select\3"
+            )
+    return line, []
+
+
 def make_template_tag_renames(line, spec):
     return _do_rename(
         line,
@@ -168,8 +180,8 @@ def flag_changed_css_classes(line, spec):
         regex = _get_direct_css_regex(css_class)
         if re.search(regex, line):
             flags.append([
-                f'css:{css_class}',
-                _get_change_guide(css_class)
+                f'css-{css_class}',
+                _get_change_guide(f'css-{css_class}')
             ])
     return flags
 
@@ -181,7 +193,7 @@ def flag_changed_javascript_plugins(line, spec):
         extension_regex = _get_extension_regex(plugin)
         if re.search(plugin_regex, line) or re.search(extension_regex, line):
             flags.append([
-                f'plugin:{plugin}',
+                f'js-{plugin}',
                 _get_change_guide(f"js-{plugin}")
             ])
     return flags
@@ -233,15 +245,24 @@ def check_bootstrap3_references_in_javascript(line):
     return issues
 
 
+def flag_file_inputs(line):
+    flags = []
+    regex = r"\btype\s*=\s*.file"
+    if re.search(regex, line):
+        flags.append([
+            "css-file-inputs",
+            _get_change_guide("css-file-inputs")
+        ])
+    return flags
+
+
 def flag_inline_styles(line):
     flags = []
     regex = r"\bstyle\s*=\s*"
     if re.search(regex, line):
         flags.append([
-            "inline style",
-            "This template uses inline styles. Please revisit this usage.\n\n"
-            "Inline styles can often be replaced with Bootstrap 5's utility classes, "
-            "particularly the spacing utilities: https://getbootstrap.com/docs/5.0/utilities/spacing/"
+            "inline-style",
+            _get_change_guide("inline-style")
         ])
     return flags
 
@@ -251,12 +272,23 @@ def flag_crispy_forms_in_template(line):
     regex = r"\{% crispy"
     if re.search(regex, line):
         flags.append([
-            "check crispy",
+            "crispy",
             _get_change_guide("crispy")
         ])
     return flags
 
 
+def flag_selects_without_form_control(line):
+    flags = []
+    if "<select" in line:
+        if "form-select" not in line and "form-control" not in line:
+            flags.append([
+                "css-select-form-control",
+                _get_change_guide("css-select-form-control")
+            ])
+    return flags
+
+
 def file_contains_reference_to_path(filedata, path_reference):
     regex = _get_path_reference_regex(path_reference)
     return re.search(regex, filedata) is not None

corehq/apps/hqwebapp/utils/bootstrap/changes_guide/css-file-inputs.md

@@ -0,0 +1,4 @@
+Please add the form-control class to this input.
+
+In Bootstrap 3, `form-control` added a border and shadow that looked weird with file inputs.
+In Bootstrap5, `form-control` is appropriate for file inputs.

corehq/apps/hqwebapp/utils/bootstrap/changes_guide/css-form-group.md

@@ -0,0 +1,10 @@
+`form-group` has been dropped. Use grid utilities instead.
+
+Since we are opting for vertical forms (where the label is directly above the field), take the following actions:
+* Remove the `div` wrapper from the `form-group`'s first child, which contains the field's label.
+* Remove the column classes (`col-lg-2`, etc.) from the `form-group`'s first child, usually a `<label>`,
+which contains the field's label. Most often, this will leave the label with just the `form-label` class.
+* Remove the column classes (`col-lg-2`, etc.) from the `form-group`'s second child, which contains the field
+control (the actual input, which will often use the `form-control` class).  Frequently, this will leave the `<div>`
+without any other classes. If so, and if it has no other attributes, it can be removed.
+* Replace `form-group` with `mb-3`.

corehq/apps/hqwebapp/utils/bootstrap/changes_guide/css-select-form-control.md

@@ -0,0 +1,4 @@
+Please replace `form-control` with `form-select`.
+
+In Bootstrap 3, selects used the `form-control` class.
+In Bootstrap5, selects use `form-select` instead.

corehq/apps/hqwebapp/utils/bootstrap/changes_guide/inline-style.md

@@ -0,0 +1,13 @@
+This template uses inline styles. Please revisit this usage.
+
+Inline styles are not best practice. While replacing them is not strictly necessary for this migration,
+Bootstrap 5 includes a host of utility classes that can replace a lot of the imline style usage in HQ.
+
+Some common replacements:
+* Display styles like `display: none` can be replaced with classes like `d-none`. See https://getbootstrap.com/docs/5.0/utilities/display/
+* Spacing styles like `margin-bottom: 10px` can be replaced with classes like `mb-3`. See https://getbootstrap.com/docs/5.0/utilities/spacing/
+* Text alignment styles like `text-align: right` can be replaced with classes like `text-end`. See https://getbootstrap.com/docs/5.0/utilities/text/
+* Usages of the `em` unit should be updated to use the `rem` unit
+* Layouts achieved using inline styling can sometimes be replaced with usage of flex. See https://getbootstrap.com/docs/5.0/utilities/flex/
+
+See all of Bootstrap 5's utility classes: https://getbootstrap.com/docs/5.0/utilities/