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

chore: do not clone bootstrap for tacc-search-bar #971

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

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Sep 20, 2024

Overview

Injecting the TACC search bar is not affected by the presence of Bootstrap, so don't load markup inside the custom element.

Related

Changes

  • removed cloning of markup that loads Bootstrap stylesheet

Testing

  1. Verify Bootstrap being loaded or not loaded has no effect on UI and UX.

UI

With Cloned Bootstrap Markup

with.Bootstrap.-.SD.480p.mov

Sans Cloned Bootstrap Markup

sans.Bootstrap.-.SD.480p.mov

Notes

Why? This may have had a purpose, and better CSS made that purpose moot. This may have had a theoretical-only purpose.

Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.48%. Comparing base (8582dfa) to head (6b91c86).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #971   +/-   ##
=======================================
  Coverage   70.48%   70.48%           
=======================================
  Files         538      538           
  Lines       33978    33978           
  Branches     2936     2936           
=======================================
  Hits        23949    23949           
  Misses       9829     9829           
  Partials      200      200           
Flag Coverage Δ
javascript 72.68% <ø> (ø)
unittests 60.80% <ø> (ø)

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

@chandra-tacc chandra-tacc self-requested a review January 9, 2025 17:52
@wesleyboar
Copy link
Member Author

wesleyboar commented Jan 14, 2025

Tip

I remember things.

Why did I have this code? The search bar is loaded as a custom HTML element. When I originally developed the search bar this way, it did not have access to Bootstrap CSS. This code I now remove added the Bootstrap CSS into the scope of the custom HTML element.

Why would it not be needed now? Uncertain. Ideas:

  • some browser might still need this
  • maybe standards/browsers have fixed the bug this hack overcame

How could I find out? Find history of similar code in TACC/Core-CMS-Custom or [(archived) taccaci/frontera-tech-docs. Are there comments in the code or its commit history? Was it removed, with explanation?

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