-
Notifications
You must be signed in to change notification settings - Fork 76
Use new Jenkins styles for Boostrap tabs #457
Conversation
Codecov Report
@@ Coverage Diff @@
## master #457 +/- ##
=========================================
Coverage 72.46% 72.46%
Complexity 1005 1005
=========================================
Files 87 87
Lines 3741 3741
Branches 435 435
=========================================
Hits 2711 2711
Misses 884 884
Partials 146 146 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| <div class="tabBarFrame"> | ||
| <ul class="flex-wrap tabBar" role="tablist" id="tab-details"> | ||
| <li class="nav-item tab" role="presentation"> | ||
| <a class="nav-link" id="overview-tab" data-bs-toggle="tab" data-bs-target="#overview" type="button" role="tab" aria-controls="overview" aria-selected="false">Overview</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do these a elements have type="button"? https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-type says the value of type is supposed to be a MIME type; "button" is not that. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#type is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because Boostrap recommends so: https://getbootstrap.com/docs/5.2/components/navs-tabs/#javascript-behavior
Does it cause any problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I changed it later on to <a> because otherwise Jenkins CSS did style those elements in a strange way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not see any problem caused by type=button, but noticed it while playing with tabindex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boostrap recommends so: https://getbootstrap.com/docs/5.2/components/navs-tabs/#javascript-behavior
On that page, I see type="button" attributes on button elements but not on a elements.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#type says it's OK for button elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as mentioned above I started with button, but then Jenkins CSS decorated these buttons so I switched back to a. I did not change the type.
Downstream PR of jenkinsci/bootstrap5-api-plugin#158.