Skip to content

UI Component Adapters - #16402

Closed
dannon wants to merge 97 commits into
galaxyproject:devfrom
dannon:component-wrappers
Closed

UI Component Adapters#16402
dannon wants to merge 97 commits into
galaxyproject:devfrom
dannon:component-wrappers

Conversation

@dannon

@dannon dannon commented Jul 14, 2023

Copy link
Copy Markdown
Member

WIP, cofest work.

This will insulate the galaxy codebase from specific external component implementations, focusing on bootstrap-vue first. Additionally, it'll allow us to customize each of these separate the underlying implementation.

  • b-alert
  • b-badge
  • b-breadcrumb*
  • b-btn
  • b-button (Ahmed)
  • b-button-group (ahmed)
  • b-button-toolbar (ahmed)
  • b-card*
  • b-carousel
  • b-dropdown
  • b-embed
  • b-form
  • b-form-checkbox
  • b-form-file
  • b-form-group
  • b-form-invalid-feedback
  • b-form-radio
  • b-form-radio-group
  • b-form-select
  • b-form-textarea
  • b-form-textarea
  • b-list-group
  • b-list-group-item
  • b-modal
  • b-nav
  • b-nav-item
  • b-overlay
  • b-popover
  • b-spinner
  • b-tab
  • b-table
  • b-table
  • b-table-lite
  • b-tabs
  • b-textarea

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@itisAliRH
itisAliRH force-pushed the component-wrappers branch 2 times, most recently from 28dd595 to e4a6c35 Compare August 10, 2023 07:29
@itisAliRH

itisAliRH commented Aug 16, 2023

Copy link
Copy Markdown
Member

All used components are wrapped (75 of them) and most of them are working as expected and their tests are passing. However, a few components still need some attention, as their tests are failing. Here's the breakdown:

  • GAlert - Tests passing ✅
  • GBadge - Tests passing ✅
  • GBreadcrumb - Tests passing ✅
  • GBreadcrumbItem - Tests passing ✅
  • GButton - Tests passing ✅
  • GButtonGroup - Tests passing ✅
  • GButtonToolbar - Tests passing ✅
  • GCard - Tests passing ✅
  • GCardBody - Tests passing ✅
  • GCardFooter - Tests passing ✅
  • GCardGroup - Tests passing ✅
  • GCardHeader - Tests passing ✅
  • GCardImg - Tests passing ✅
  • GCardText - Tests passing ✅
  • GCardTitle - Tests passing ✅
  • GCarousel - Tests passing ✅
  • GCarouselSlide - Tests passing ✅
  • GCol - Tests passing ✅
  • GCollapse - Tests passing ✅
  • GContainer - Tests passing ✅
  • GDropdown - Tests passing ✅
  • GDropdownDivider - Tests passing ✅
  • GDropdownForm - Tests passing ✅
  • GDropdownGroup - Tests passing ✅
  • GDropdownItem - Tests passing ✅
  • GDropdownText - Tests passing ✅
  • GEmbed - Tests passing ✅
  • GForm - Tests passing ✅
  • GFormCheckbox - Tests failing ❌
  • GFormCheckboxGroup - Tests failing ❌
  • GFormDatalist - Tests passing ✅
  • GFormDatepicker - Tests passing ✅
  • GFormFile - Tests passing ✅
  • GFormGroup - Tests passing ✅
  • GFormInvalidFeedback - Tests passing ✅
  • GFormRadio - Tests failing ❌
  • GFormRadioGroup - Tests failing ❌
  • GFormSelect - Tests passing ✅
  • GFormSelectOption - Tests passing ✅
  • GFormText - Tests passing ✅
  • GFormTextarea - Tests passing ✅
  • GImg - Tests passing ✅
  • GInput - Tests passing ✅
  • GInputGroup - Tests passing ✅
  • GInputGroupAppend - Tests passing ✅
  • GInputGroupPrepend - Tests passing ✅
  • GInputGroupText - Tests passing ✅
  • GLink - Tests passing ✅
  • GListGroup - Tests passing ✅
  • GListGroupItem - Tests passing ✅
  • GModal - Tests passing ✅
  • GNav - Tests passing ✅
  • GNavbar - Tests passing ✅
  • GNavbarBrand - Tests passing ✅
  • GNavbarNav - Tests passing ✅
  • GNavItem - Tests passing ✅
  • GNavItemDropdown - Tests passing ✅
  • GOverlay - Tests passing ✅
  • GPagination - Tests passing ✅
  • GPopover - Tests failing ❌
  • GProgress - Tests passing ✅
  • GProgressBar - Tests passing ✅
  • GRow - Tests passing ✅
  • GSpinner - Tests passing ✅
  • GTab - Tests passing ✅
  • GTable - Tests passing ✅
  • GTableLite - Tests passing ✅
  • GTableSimple - Tests passing ✅
  • GTabs - Tests passing ✅
  • GTbody - Tests passing ✅
  • GTd - Tests passing ✅
  • GTh - Tests passing ✅
  • GThead - Tests passing ✅
  • GTooltip - Tests passing ✅
  • GTr - Tests passing ✅

@ElectronicBlueberry

Copy link
Copy Markdown
Member

I think that the popover tests can not be fixed, because stubbing composition components does not seem to work in v1 of vue test utils. It needs to be stubbed, because B-Popover renders it's content outside of its parents dom node, which is not something JSDOMEnvironment seems to be able to handle. We can't upgrade to vue test utils 2, since that requires vue 3, which requires this PR. I'd recommend removing it from the PR for now, and handling this in a separate PR, where instead of wrapping the component, we directly replace it with a custom component that renders it's contents withing it's parents dom node (such as dialog).

@ElectronicBlueberry

Copy link
Copy Markdown
Member

While trying to fix the remaining failing tests, I noticed a bigger issue which these tests uncover.

Most of the tests fail, because the test is expecting to select the inner element, but selects the wrappers div instead. This is due to v-bind forwarding, and self-applying some attributes. For example, have a look at this output html from one of the failing tests:

              <div id="confirmation-modal" style="position: absolute; z-index: 1040;" title="Permanently delete 2 items?" title-tag="h2" ok-title="Permanently delete" ok-variant="" ok-disabled="true" static="" centered="">
                  <transition-stub enterclass="" leaveclass="" entertoclass="" leavetoclass="" enteractiveclass="" leaveactiveclass="">
                    <div id="confirmation-modal" role="dialog" aria-hidden="true" aria-labelledby="confirmation-modal___BV_modal_title_" aria-describedby="confirmation-modal___BV_modal_body_" class="modal fade" style="display: none;">

Notice the same idappearing twice.

This is an issue beyond just the failing tests, since duplicate ids are invalid html.

@martenson martenson added the kind/refactoring cleanup or refactoring of existing code, no functional changes label Sep 12, 2023
@dannon

dannon commented Mar 20, 2026

Copy link
Copy Markdown
Member Author

Closing this -- the approach here (thin wrappers re-exporting Bootstrap-Vue) has been superseded by the component-by-component replacement work tracked in #21956. The current PRs (#21957, #21958, #21959, #21960, #21961, #21962) create real custom implementations using CSS transitions, @floating-ui/dom, etc. rather than pass-through adapters, which is necessary since the scoped-slot-based Bootstrap-Vue components don't work under @vue/compat. The new components live in src/components/BaseComponents/ and are landing incrementally in focused, reviewable PRs.

This PR was directionally right about the need to decouple from Bootstrap-Vue, but the execution path has changed. Leaving the branch in case anything is useful for reference.

@dannon dannon closed this Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/admin area/documentation kind/refactoring cleanup or refactoring of existing code, no functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants