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

Migrate to Bootstrap 5 #1882

Merged
merged 11 commits into from
Apr 15, 2022
Merged

Migrate to Bootstrap 5 #1882

merged 11 commits into from
Apr 15, 2022

Conversation

jovyntls
Copy link
Contributor

@jovyntls jovyntls commented Apr 5, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Fixes #1702

Overview of changes:
Migrate Bootstrap to v5.1.3

Anything you'd like to highlight / discuss:
Breaking change to MarkBind: text-dark now needs to be manually applied on badges with bg-warning, bg-info, bg-light (previously text would automatically be dark for these classes).

Testing instructions:

  • Full functionality should be supported
  • No disruptive changes in the UI
    • Major UI changes should be listed below

Proposed commit message: (wrap lines at 72 characters)

Migrate Bootstrap to v5.1.3

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Breaking Changes (for MarkBind)

  • text-dark must be manually applied for bg-warning, bg-info, bg-light (previously text would automatically be dark for these classes).

Notable UI Changes

Before After
Theme colours: Lower contrast (applies to any component using Bootstrap theme colours, e.g. badges, tipboxes) Increased contrast. See badges for comparison
Badges: increased padding
Screenshot 2022-04-09 at 1 50 23 AM Tables: Divider between table header and body Screenshot 2022-04-09 at 1 50 11 AM Less padding in tables
image Panels: Slimmer panel, less padding image
image Popovers: Border below popover headers; increased padding image
No RFS (Responsive Font Sizes) RFS support: font sizes are now scaled relative to the viewport, instead of absolute. (e.g. h1 on a smaller screen will be smaller than h1 on a larger screen)

Breaking Changes in Bootstrap 4 -> 5

This list serves to account for all the changes needed/made for this migration.
Full list of breaking changes from Bootstrap 4->5 may be found in the Bootstrap migration docs.

Utilities

Components

  • Accordion (new component - no action needed)
  • Alerts (not used - no action needed)
  • Badges (Bootstrap 5: Migrate badges #1884)
    • .badge-* color classes -> .bg-*
    • .badge-pill -> .rounded-pill
  • Breadcrumbs (not used - no action needed)
  • Buttons (breaking changes won't affect us)
  • Card (breaking changes won't affect us)
  • Carousel (not used - no action needed)
  • Close button (Bootstrap 5: Close button #1886)
  • Collapse (not used - no action needed)
  • Dropdowns (Bootstrap 5: Dropdowns #1888)
  • Jumbotron (Bootstrap 5: Remove jumbotron class #1885)
    • not supported by MarkBind officially, only used for styling docs
  • List group (only new additions - no action needed)
  • Navs and tabs (only new additions - no action needed)
  • Navbars (Bootstrap 5: Prevent Navbars from being printed #1906)
  • Offcanvas (new component - no action needed)
  • Pagination (only new additions - no action required)
  • Popovers (not used - no action needed)
  • Spinners (not used - no action needed)
  • Toasts (not used - no action needed)
  • Tooltips (not used - no action needed)

Sass

  • (not used in MarkBind, no action needed)

Color system

  • (not used in MarkBind, no action needed)

Grid updates

Content, Reboot, etc

RTL

  • Horizontal direction specific variables, utilities, and mixins have all been renamed to use logical properties like those found in flexbox layouts—e.g., start and end in lieu of left and right.

Forms

  • Dropped classes are not used anywhere in MarkBind

Helpers

  • All changes do not affect MarkBind

JavaScript

Other changes required

@jonahtanjz
Copy link
Contributor

Indirectly affected: to check manually afterwards

Not sure if you can see it but on the mobile navbar, will need to remove the spacing at the sides. Probably related to the grid updates.

image

Will also need to give some padding to the navbar in general.

Before:
image

After (A little bit tight around the edges):
image

@jonahtanjz
Copy link
Contributor

Just a heads up. Since this PR involves multiple changes with organised commits, we will be using the merge commit strategy (all commit messages will be added to the master branch). Would be good to remove these "merge commits" :)

image

@jovyntls
Copy link
Contributor Author

Just a heads up. Since this PR involves multiple changes with organised commits, we will be using the merge commit strategy (all commit messages will be added to the master branch). Would be good to remove these "merge commits" :)

I see, thanks for the info! I'll rebase this branch on top of the master branch to avoid merge conflicts.

In Bootstrap 5, `<hr>` elements now use height instead of border to
better support the size attribute. However, since MarkBind's hr styles
support dotted/dashed/double lines, `<hr>` needs a border-based
implementation. 

Let's minimally override Bootstrap 5's `<hr>` styling to continue
supporting styled horizontal rules.
@jovyntls jovyntls changed the title [WIP] Migrate to Bootstrap 5 Migrate to Bootstrap 5 Apr 15, 2022
@jovyntls jovyntls marked this pull request as ready for review April 15, 2022 07:57
@jonahtanjz
Copy link
Contributor

jonahtanjz commented Apr 15, 2022

Overall looks good @jovyntls!

Just one regression:

Navbars (breaking changes won't affect us)

Seems like the navbar in bootstrap 5 is included during printing.

image

Will need to add the d-print-none class to the navbar.

@jovyntls
Copy link
Contributor Author

Thank for the catch @jonahtanjz, I forgot about the printing :o I've opened #1906 to address this!

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Nice work @jovyntls!

Overall looks good on my end. Thanks again for tackling this :)

LGTM 👍🎉

@jonahtanjz jonahtanjz added this to the 4.0 milestone Apr 15, 2022
@jonahtanjz jonahtanjz merged commit 6a7ea88 into master Apr 15, 2022
@jonahtanjz jonahtanjz deleted the bootstrap-v5-migration branch April 15, 2022 15:52
kaixin-hc added a commit to kaixin-hc/markbind that referenced this pull request Apr 18, 2022
commit 32f2e64
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Mon Apr 18 01:11:00 2022 +0800

    Fix typos in docs (MarkBind#1902)

commit cac6027
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Sun Apr 17 22:34:15 2022 +0800

    Remove package-lock file in packages/core (MarkBind#1908)

commit 6a7ea88
Merge: 30e3637 8106d48
Author: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com>
Date:   Fri Apr 15 23:52:47 2022 +0800

    Migrate Bootstrap to v5.1.3 (MarkBind#1882)

commit 8106d48
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Fri Apr 15 23:31:54 2022 +0800

    Prevent Navbars from being printed (MarkBind#1906)

commit 30e3637
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Fri Apr 15 18:40:55 2022 +0800

    Update CI steps to set up Graphviz and Java for PlantUML support (MarkBind#1905)

    PlantUML feature requires both Java and Graphviz to be installed.
    Current CI steps do not guarantee the presence of these
    dependencies. This means some PlantUML diagrams were
    unable to be generated correctly.

    Let's update our CI steps to ensure both Java and Graphviz are
    available in the build image to support PlantUML syntax.

    This will ensure usage of PlantUML syntax can be supported in
    our tests and our documentation.

commit d7c6819
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Fri Apr 15 12:35:54 2022 +0800

    Migrate Bootswatch themes to v5.1.3 (MarkBind#1898)

commit aadc9b6
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Wed Apr 13 16:25:34 2022 +0800

    Refactor includePanelProcessor and MdAttributeRenderer (MarkBind#1896)

commit 33383ca
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Wed Apr 13 01:11:23 2022 +0800

    Migrate horizontal rules (MarkBind#1901)

    In Bootstrap 5, `<hr>` elements now use height instead of border to
    better support the size attribute. However, since MarkBind's hr styles
    support dotted/dashed/double lines, `<hr>` needs a border-based
    implementation.

    Let's minimally override Bootstrap 5's `<hr>` styling to continue
    supporting styled horizontal rules.

commit eef4089
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Wed Apr 13 01:10:58 2022 +0800

    Fix spacing for navbars and searchbars (MarkBind#1900)

commit 84099ac
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 12 15:05:17 2022 +0800

    Migrate MarkBind's quizzes and questions (MarkBind#1890)

commit 5485206
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 12 13:23:14 2022 +0800

    Namespace Bootstrap data attributes with data-bs- (MarkBind#1892)

commit 0ad144d
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 12 11:57:14 2022 +0800

    Migrate dropdown components (MarkBind#1888)

commit 5f9cc95
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 12 00:02:32 2022 +0800

    Remove jumbotron class (MarkBind#1885)

commit c1436a1
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Sat Apr 9 11:53:07 2022 +0800

    Migrate close buttons (MarkBind#1886)

commit 7bc7878
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Thu Apr 7 15:46:51 2022 +0800

    Migrate breaking changes for badges (MarkBind#1884)

commit e35c32b
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 5 21:15:24 2022 +0800

    Migrate utilities to Bootstrap V5 (MarkBind#1834)

commit 9d1dcdd
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Tue Apr 12 11:56:53 2022 +0800

    Update GitHub issue template checkbox to dropdown (MarkBind#1897)

commit 950b422
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Sat Apr 9 18:15:17 2022 +0800

    Fix UG display issues when using the outputBox boilerplate (MarkBind#1889)

commit 7608553
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Thu Apr 7 15:10:37 2022 +0800

    Add an explanation on how to use JavaScript chart libraries (MarkBind#1872)

    Charts can be useful in MarkBind sites to display data.

    Let's include a section in our UG to highlight the existing
    (basic) support for JavaScript chart libraries in MarkBind.

    This is the most flexible option that MarkBind users can
    go for to utilize a JavaScript chart library of their choice.

commit f14ea43
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Wed Apr 6 01:59:22 2022 +0800

    Fix edge cases of link validation and conversion (MarkBind#1875)

    Invalid intralinks have been reported in MarkBind docs. The
    reasons are 1. actual invalid link 2. file path contains a space
    3. Email link wrongly validated.

    Let's fix all three link validation & conversion errors.
    File paths with %20 will be correctly checked for
    existence. Email(tel) links will no longer be appended
    with the base URL or validated as intralink.

    The removal of invalid link behavior will allow
    the link & link validation mechanism to function
    properly.

commit 4f060c6
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Tue Apr 5 21:50:39 2022 +0800

    Update CI to run on all PR events (MarkBind#1878)

commit 7673e66
Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com>
Date:   Tue Apr 5 21:44:19 2022 +0800

    Update documentation on MarkBind GitHub Actions (MarkBind#1880)

    The existing GitHub Action "markbind-action" has been
    revamped to improve its functionalities.

    Let's add a reference to "markbind-action" in our
    user guide to highlight this helper workflow. Let's also
    include a section in our developer guide to explain how
    to develop and maintain the "markbind-action" repo.

    This helps users discover "markbind-action" and use it in
    their CI/CD workflow. The dev docs will help developers
    continue to improve "markbind-action"

commit feee60a
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Tue Apr 5 11:19:51 2022 +0800

    Remove bootstrap-vue (MarkBind#1873)

    bootstrap-vue is no longer used in MarkBind as it relies on Vue 2 and
    Bootstrap 4, both which we plan to upgrade. Its components have been
    migrated to other libraries.

    Let's remove all mentions and instances of bootstrap-vue from MarkBind
    to fully remove this dependency.

commit 1bd81ff
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Mon Apr 4 13:35:06 2022 +0800

    Add Components page to developer guide (MarkBind#1865)

    Add Writing Components page to dev docs

commit dac728d
Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com>
Date:   Mon Apr 4 12:00:48 2022 +0800

    Migrate popovers and tooltips to floating-vue (MarkBind#1868)

    Popovers and tooltips are implemented using bootstrap-vue.

    bootstrap-vue has dependencies on both Vue and Bootstrap, and has not
    migrated the latest versions. This is a blocker for MarkBind's
    migration of these dependencies.

    Let's use floating-vue's Menu and Tooltip components for Popovers and
    Tooltips respectively. floating-vue supports both Vue 2 and Vue 3, and
    has fewer dependencies. This allows us to migrate to Vue 3 and
    Bootstrap 5 more easily in the future.
@jonahtanjz jonahtanjz added the breakingChange 💥 Feature will behave significantly different, or is made obsolete label Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breakingChange 💥 Feature will behave significantly different, or is made obsolete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bootstrap 5
2 participants