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: wp-895 menu bugs since bootstrap 5 #1073

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Mar 4, 2025

Overview

  1. Do not let Portal Nav dropdown menu be positioned off-screen horizontally.
    Fix moved to Fix: Workbench drop down menu is cut off on the right. #1066.
  2. Fix CMS dropdown menu not opening.
  3. Fix navbar toggle not working (narrow screen).

Important

Upon merge, notify @shayanaijaz, so he can update Digital Rocks Portal branch(es).

Related

Changes

  • changed classes from Bootstrap4 to Bootstrap 5

Testing

  1. Be in Portal (not on a CMS page).

3. Portal Nav Dropdown Off-Screen

  1. Click portal nav dropdown menu.
  2. Verify dropdown menu is not off the right side of the window.

Also fixed in #1066.

4. CMS Dropdown Menu Not Opening

  1. Create/Find a CMS menu that with a dropdown.
  2. Click dropdown menu button.
  3. Verify dropdown menu opens.

5. Fix Navbar Toggle Not Working

  1. Make screen narrow enough to hide menu.
  2. Verify menu toggle opens menu.

UI

3. Portal Nav Dropdown Off-Screen

fix.nav.position.more.mov

Also fixed in #1066.

4. CMS Dropdown Menu Not Opening

Fixed

cms.dropdown.menu.dev-cep.works.mov

Warning

Unable to test locally because of WP-897.

Broken

cms.dropdown.menus.BROKEN.mov

5. Fix Navbar Toggle Not Working

Fixed

navbar.expand-collapse.FIXED.mov

Broken

navbar.expand-collapse.BROKEN.mov

Copy link

codecov bot commented Mar 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.34%. Comparing base (c210828) to head (e8970c7).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1073   +/-   ##
=======================================
  Coverage   70.34%   70.34%           
=======================================
  Files         538      538           
  Lines       33328    33328           
  Branches     2953     2953           
=======================================
  Hits        23446    23446           
  Misses       9684     9684           
  Partials      198      198           
Flag Coverage Δ
javascript 72.53% <ø> (ø)
unittests 61.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Notes for reviewers.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file is not used. The file itself stated that.1

Footnotes

  1. I had kept it around for parity with Core-CMS (Django CMS), but I know now Portal is just Django (not Django CMS).

Comment on lines +25 to +41

/* Make (CMS) Bootstrap 4 toggle compatible with (Portal) Bootstrap 5 */
[ ...container.querySelectorAll('[data-toggle]')].forEach(toggle => {
const cmsUsesBootstrap4Toggle = (
toggle.dataset.toggle !== undefined &&
toggle.dataset.bsToggle === undefined
)

if ( cmsUsesBootstrap4Toggle ) {
toggle.dataset.bsToggle = toggle.dataset.toggle;
delete toggle.dataset.toggle;
console.log(
'Replaced `data-toggle` with `data-bs-toggle` in `#s-cms-nav`.',
'To not need this, update CMS Bootstrap from 4 to 5.'
)
}
});
Copy link
Member Author

Choose a reason for hiding this comment

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

@wesleyboar wesleyboar requested review from taoteg and fnets March 4, 2025 00:43
Copy link
Contributor

@fnets fnets left a comment

Choose a reason for hiding this comment

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

Works locally, and LGTM

wesleyboar added a commit to TACC/Core-CMS that referenced this pull request Mar 4, 2025
PR TACC/Core-Portal#1073/1066 fixes portal nav dropdown menu alignment.

But that breask alignment for CMS, because CMS uses old Bootstrap.
wesleyboar added a commit to TACC/Core-CMS that referenced this pull request Mar 4, 2025
* fix: WP-894 portal nav dropdown menu alignment

PR TACC/Core-Portal#1073/1066 fixes portal nav dropdown menu alignment.

But that breask alignment for CMS, because CMS uses old Bootstrap.

* refactor: make fix independent of similar fix

* fix: missing punctuation
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.

2 participants