Adopt GButton across remaining components, retire BButton usage#22688
Draft
dannon wants to merge 18 commits into
Draft
Adopt GButton across remaining components, retire BButton usage#22688dannon wants to merge 18 commits into
dannon wants to merge 18 commits into
Conversation
Replaces BButton imports and tags with GButton in seven components that use only static variant/size strings. No behavior change -- GButton renders the same <button> with equivalent styling. Part of the bootstrap-vue elimination sweep.
Replaces BButton imports and tags with GButton across thirteen Workflow editor, list, run, published, invocation, and metrics components that use only static variant/size strings. No behavior change. Part of the bootstrap-vue elimination sweep.
Replaces BButton imports and tags with GButton across thirteen History components (archiving, content/dataset actions, navigation, list, layout, multi-view) that use only static variant/size strings. No behavior change. Part of the bootstrap-vue elimination sweep.
Replaces BButton with GButton across two Dataset components and five Libraries/LibraryFolder components using only static variant/size strings. FolderDetails (uses v-b-modal directive) deferred to a follow-up commit. No behavior change.
Replaces BButton with GButton in two admin form components and three ConfigTemplates components using only static variant/size strings. DataManagerJobs (uses dynamic :variant) deferred to a follow-up. No behavior change.
Replaces BButton with GButton across ten components using only static variant/size strings: Markdown rendering, Sharing dialog and embeds, SelectionDialog, SchemaOrg creator editor, and TagsMultiselect. CellButton (dynamic) deferred. No behavior change.
…c usages Replaces BButton with GButton across thirteen components in Tool, ToolsList, Panels, Notifications, PageDisplay, User Notifications, InteractiveTools, JobInformation, and Collections/sheet that use only static variant/size strings. JobOutputs.vue's block prop becomes class="w-100". Dynamic-variant components deferred. No behavior change.
Five icon-only buttons converted from variant="link" in the previous Panels/Interactive/Collections sweep were missing the transparent prop. Without it GButton falls back to grey background and border, which differs visually from BButton's link variant. This restores the no-background, no-border look.
Replaces BButton with GButton in eight components originally miscategorized as dynamic-variant -- their :variant= bindings were on BAlert/BBadge rather than BButton, so the BButtons themselves use static variant strings only. Covers Login, admin/DataManager, Tool/User credentials, Upload/RulesInput, Workflow run, CustomBuilds, and Grid/GridList sort headers.
Adds client/src/components/BaseComponents/variantToColor.ts as a shared helper for translating bootstrap-vue variant strings to GButton color/outline/transparent attributes, plus a sizeToGSize helper for size translation. Uses it across eight components that bind :variant= dynamically: CellButton, BroadcastContainer, ObjectStoreSelectButton, CompositeBox, RulesInput, ServiceCredentials, GCard, HistoryCounter, and MultipleViewItem. Static buttons in the same files use direct prop translation. No behavior change.
Replaces the BButton + v-b-modal directive trigger with a GButton
that explicitly calls $bvModal.show('details-modal') on click. The
BModal itself stays put -- this PR is only about retiring BButton,
and the bootstrap-vue plugin still exposes $bvModal for programmatic
modal control. Functionally equivalent to the directive.
Test updates: - swap btn-primary / btn-outline-primary class selectors for g-button g-blue / g-blue g-outline (DatasetCopy) - swap "active" pressed-state class for "g-pressed" (FormData) - swap "btn" class check for "g-button" (FilterMenu) - swap "disabled" attribute check for "aria-disabled" -- GButton drops the native disabled attribute in favor of aria-disabled to keep the click handler reachable by screen readers (FilesDialog) - swap shallow-mount BButton stub names for GButton (RoleForm, PageForm, ConfigureHeader, ExternalRegistration) - mount instead of shallowMount where shallow stubs swallowed click events (SharingIndicators, SelectionStatus, HistoryCounter partial via direct findComponent emit) - inject a test router into LibraryDataset so the GButton-rendered router-link can resolve Production fixes uncovered by the test sweep: - CellButton: GButton does not forward template listeners through v-bind="$attrs", so add .native modifier on @mouseleave/@blur to keep the auto-blur-on-mouseleave behavior working - LibraryDataset: convert the named-route :to object to a path template literal since GButton.to is typed as string and the object form was triggering a Vue prop type warning
Two safety fixes to GButton itself, motivated by the wider BButton adoption sweep. First, default the rendered `<button>` element to `type="button"` when no explicit type is provided via $attrs. Without this, any GButton inside a `<form>` silently defaults to type="submit" and starts submitting the form on click -- a regression Bootstrap's BButton avoided by setting type="button" itself. Explicit `type="submit"` / `type="reset"` from callers still wins via Vue's later-binding precedence on `v-bind="$attrs"`. Second, add `.g-pressed` styling for solid (non-outline, non-transparent) GButtons. Pressed state previously only rendered for outline and transparent variants, so solid buttons with `:pressed` had no visual indication. Grey solid pressed now uses --color-grey-400 background and --color-grey-600 border, matching the existing hover/active direction; colored solid pressed uses the darker --color-*-700 / --color-*-900 pair.
Cleanup of direct b-button / BButton usages that the earlier static and dynamic sweeps missed -- mostly lowercase `<b-button>` forms (the prior commits only matched the PascalCase tag) across markdown and page editors, library permissions pages, API key management, schema.org forms, user-facing pages, tool shed install actions, workflow attributes/run/storage, and the export ActionButton. Standard prop translations apply: variant=primary -> color=blue, variant=link -> transparent, size=sm -> size=small, etc. WorkflowAttributes' Readme toggle button keeps its :pressed binding; the matching solid-pressed styling lives in the prior GButton commit.
Replaces BButtonGroup wrappers with GButtonGroup across the components whose children were converted to GButton in this branch. Bootstrap's .btn-group child styling targets .btn classes, so leaving GButton children inside a BButtonGroup wrapper produces ungrouped, individually- rounded buttons -- a visible regression. GButtonGroup already exists with the equivalent corner-merge selectors targeting .g-button. A few of these files (ZoomControl, FormColumnDefinitions, FormRecordFieldDefinitions, Workflow/Editor/Index) also still had lowercase <b-button> children inside their wrappers; those get converted to GButton in the same change since wrapper migration required it. The remaining BButtonGroup in WorkflowInvocationMetrics.vue is left as BButtonGroup intentionally -- it wraps BDropdown triggers (still .btn- based), not GButtons, so the .btn-group child styling still applies. Also updates client/src/components/Workflow/Editor/Comments/_buttonGroup.scss to target the new .g-button.g-outline.g-blue and .g-pressed class combinations that GButton renders instead of Bootstrap's .btn-outline-primary / .active.
The previous `:type="baseComponent === 'button' && !$attrs.type ? 'button' : undefined"` relied on `v-bind="$attrs"` to re-apply caller-provided `type="submit"` / `type="reset"`. In Vue 2 that does not work: explicit attribute bindings take precedence over `v-bind="object"` regardless of source order, so the explicit `:type="undefined"` won and the rendered button had no type attribute at all. The form-button HTML default still made it act as submit at runtime, but CSS selectors like `button[type='submit']` stopped matching -- breaking tests in LoginForm, RegisterForm, and ChangePassword that find their submit button by attribute. Computing the type directly via `$attrs.type ?? 'button'` makes the explicit binding render whichever value is right, no longer dependent on `v-bind="$attrs"` precedence.
The FormDataContextButtons wrapper switched from BButtonGroup (which rendered `.btn-group`) to GButtonGroup (which renders `.g-button-group`) as part of the wider button-group migration. Three find selectors in the FormData test were still scoping by `.btn-group`, which now matches nothing in the rendered output.
The branch's BButton variant="link" -> GButton transparent translation
consistently dropped the link-color part of Bootstrap's link variant.
GButton's transparent (no color) renders as --color-grey-700 (#35373f,
near-black), while variant="link" originally rendered in Bootstrap's
link blue (Galaxy's --color-blue-600 / #25537b navy). The plan's
translation table flagged this -- "transparent (no color, or color=blue
if styled blue)" -- but every conversion picked the no-color option.
Audit: pre-existing GButton callers on upstream/dev (Tool/Buttons,
PairedElementView, FormDrilldownOption, etc.) follow the convention
`<GButton transparent color="blue">` for styled-blue text/link buttons
and `<GButton transparent>` (no color) for icon-only buttons matching
surrounding text. This commit applies that convention retroactively.
Static call-sites (visible text content, was variant="link"): adds
color="blue" to the sort-header in Grid/GridList, the display-btn
showing sub-item counts in ContentItem, both nav buttons in
CollectionNavigation, the three filter buttons (active/deleted/hidden)
in HistoryCounter, the "History" button in HistoryNavigation, "Select
All" in SelectionStatus, both Library permissions "Go back" buttons,
and the Simple Form toggle in WorkflowRunForm.
Dynamic helper: variantToColor("link") now returns
{ transparent: true, color: "blue" } so HistoryCounter's refresh button
keeps its navy color in the link state (and falls back to red via the
existing danger branch). HistoryCounter.test.ts assertions updated to
match.
Icon-only conversions (ContentOptions toolbar, DatasetActions, Sharing
indicators, Tag delete, workflow move/delete buttons, page editor
controls, modal close X's, etc.) are intentionally left as transparent
without color -- those were always meant to inherit the surrounding
text color and the no-color render is correct for them.
dannon
force-pushed
the
refactor/adopt-gbutton-everywhere
branch
from
7fed844 to
8abfa7c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finishes the BButton → GButton sweep. Galaxy already had a custom
GButtonatclient/src/components/BaseComponents/GButton.vueused by 131 files; this PR converts the remaining direct usages across ~100 components (plus tests) so every<BButton>/<b-button>is now a<GButton>. After this,rg -n "<(b-button|BButton)(\s|>)" client/src --glob '*.vue'returns zero hits.The immediate motivation is the Vue 3 migration branch, which is blocked by ~44 failing tests where bootstrap-vue's
BButtondropsclassanddata-*attributes through its compat-mode functional-component shim.GButtondoesn't have that bug because it uses real Vue 3 templates withv-bind="$attrs". Independent of the migration, every file off BButton is one less thing to drag along when bootstrap-vue is eventually removed -- continuation of the same sweep that merged GTabs (#21960), GCollapse (#21958), GOverlay (#21957), and v-g-tooltip (#21962).This is an adoption sweep, not a new component. Mostly
s/BButton/GButton/with prop translations per the planning notes. A few changes go wider:GButton: default native
type="button". Without this, any<GButton>inside a<form>defaults totype="submit"per HTML5 spec and silently starts submitting the form on click -- a regression Bootstrap's BButton avoided by settingtype="button"itself. Explicittype="submit"/type="reset"from callers still wins via the Vue template's attribute-binding precedence. (One subtle Vue 2 gotcha: explicit attribute bindings take precedence overv-bind="$attrs"regardless of source order, so the type value has to be computed inline rather than relying on$attrsto override anundefinedfallback.)GButton: solid
.g-pressedstyling. GButton previously had.g-pressedrules only for the outline and transparent variants, so a solid<GButton :pressed>had no visual pressed state. Added grey and color solid-pressed styles using the same--color-*-700/--color-*-900direction as existing hover-active rules.BButtonGroup→GButtonGroupmigration. Bootstrap's.btn-groupcollapses borders and rounds only the outer corners by targeting.btnchildren. With GButton children (.g-button), those selectors don't match, so groups visually de-fuse. Galaxy already has aGButtonGroupcomponent with equivalent child-targeting; this PR migrates 17 wrappers across the codebase. One BButtonGroup remains inWorkflowInvocationMetrics.vue-- it groups BDropdown triggers (still.btn-based), so its child styling still applies until BDropdown gets a G-equivalent.Preserving link color on
variant="link"conversions. Bootstrap'svariant="link"rendered button text in the themed link blue (--color-blue-600, #25537b navy). GButton'stransparent(no color) defaults to--color-grey-700(#35373f, near-black), so converted link buttons looked wrong in places like the history filter buttons, the Library "Go back" links, the workflow Simple Form toggle, etc. Audited the 71 transparent conversions in this diff and addedcolor=\"blue\"to the 12 with visible text content (and updatedvariantToColor(\"link\")for the one dynamic case in HistoryCounter's refresh button). Icon-only transparent buttons (close-X's, toolbar icons, etc.) are correctly left without color -- they're meant to inherit surrounding text color, matching the pre-existing convention in the 130 files already using GButton.Outstanding items worth flagging
LibraryDataset.vuehad a named-routeto=\"{ name: 'LibraryFolderDatasetPermissions', params: {...} }\"that became a hardcoded path template literal becauseGButton.tois typed asstring. If the route ever moves, this silently breaks. Follow-up should widen the type toRouteLocationRawor similar.CellButton.vueuses@mouseleave.native/@blur.nativebecause GButton doesn't forward template listeners throughv-bind=\"$attrs\". Works under Vue 2 compat; will need attention during the Vue 3 migration since.nativeis removed.WorkflowInvocationMetrics.vueretains one BButtonGroup as noted above; convertable once BDropdown follows.Test plan
pnpm test(2013 passed / 2 skipped / 0 failed)pnpm type-checkcleanpnpm eslint --quietclean