Restore styling and behaviour lost in the GButton adoption sweep - #23464
Draft
itisAliRH wants to merge 37 commits into
Draft
Restore styling and behaviour lost in the GButton adoption sweep#23464itisAliRH wants to merge 37 commits into
itisAliRH wants to merge 37 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.
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.
These files were added to dev after the original GButton migration was written, so rebasing left them still on bootstrap-vue's BButton. Convert them to GButton following the same variant->color mapping the rest of the PR uses (link -> transparent blue, outline-primary -> blue outline, success -> green, outline-danger -> red outline, sm -> small). ProposalDiffView and SectionPatchView's tests were shallow-mounting, which worked with BButton (functional, native listeners) but not with GButton, which emits click from its own root handler. Mount those two for real, and check aria-disabled instead of the native disabled attribute since that's how GButton signals disabled state.
SelectPreferredStore.test.ts finds the modal's Ok button with ".g-button.g-blue". That was unambiguous while GCard's action buttons were still BButtons, but converting them means variant="outline-primary" now renders .g-button.g-blue too, so find() returned a card action instead and the click never reached the modal. Scope the selector to .g-modal-confirm-buttons, which is the footer GModal renders.
The comment explains why the refresh button needs a logged-in user to render. HistoryCounter switched to GButtonGroup in this PR, so the comment was naming a component the file no longer uses.
A native disabled button dispatches no click event at all. GButton and GLink deliberately render `aria-disabled` instead of the native attribute so a disabled control stays hoverable for its `disabledTitle` tooltip, but the JS guard in `onClick` only called `preventDefault()`. The native click therefore kept bubbling: clickable ancestors (history content rows, GCard roots, sidebar list rows) still reacted to clicks on disabled buttons, and any `.stop`/`.prevent` modifier a caller put on the component never ran, because those modifiers are attached to the component listener that is skipped when disabled. Call `stopPropagation()` alongside `preventDefault()` so a disabled control behaves like a native one.
When `to` is set the root element becomes a RouterLink. Vue 2 treats `@click` on a component as a component listener, and vue-router 3 neither emits a `click` event nor merges `$listeners`, so the `onClick` handler never ran and every caller-supplied `@click` was silently dead. Live casualties include GCard's primary and secondary actions, the tool list upload action and SourceOptionCard's select emit. Bind `@click.native` alongside the existing `@click`. Vue 2 ignores `nativeOn` on a plain element root, and binds it to the rendered anchor for a component root, so exactly one of the two listeners fires in every case. Both collapse to a single `@click` once the components run on Vue 3. Also refresh the `useClickableElement` docstring, which justified the plain `button` root for the disabled state with the now-fixed listener gap.
The root rendered a bare `div`, so the `aria-label` callers pass (for
instance MultiviewPanel's pin-recent group) was not exposed to assistive
technology; BButtonGroup rendered `role="group"`.
The group also lacked Bootstrap's `.btn-group > .btn { flex: 1 1 auto }`,
so a group stretched with `w-100` left its buttons at content width.
Bootstrap's `.btn` centred its label with `text-align: center`, which has
no effect on the `inline-flex` box `.g-button` uses. Full-width buttons
("Show N more outputs", the workflow readme toggle) rendered left-aligned,
and the fixed 1.5rem glyph buttons in the workflow comment toolbars sat at
flex-start inside their square.
Bootstrap's radius and z-index rules for an input group only ever matched `.btn`, so a GButton in an `.input-group-append` kept its own rounded left corners and rendered a visible seam against the adjoining input, and its focus ring was painted under the control instead of over it. Add the mirror-image rules for `.input-group-prepend` while we are here.
`&.g-pressed:not(.g-outline):not(.g-transparent)` is more specific than the sibling `&:hover, &:focus-visible` block, so a pressed solid button lost all hover and keyboard-focus feedback and read as inert. Restate the feedback inside the pressed blocks, one step darker than the pressed background.
The rename pencil, the bookmark star and the card indicators carry `.inline-icon-button`, a global helper that painted them brand blue and borderless. That helper is a single class selector, so it now loses to GButton's scoped variant rules and the buttons render grey with a grey hover fill. Say it with props instead: `transparent icon-only color="blue"` on the pencil and star, and a `link` default variant for indicators, which `variantToColor` maps to the same transparent blue while still honouring an explicit `indicator.variant`.
The Edit URL / Done / Copy buttons kept the `.inline-icon-button` helper but gained no GButton props in the sweep, so they went from borderless brand-blue glyphs to full grey bordered buttons: the helper is a single class selector and loses to GButton's scoped variant rules.
The paging arrows relied on `.inline-icon-button` for their brand-blue borderless look and now render as grey bordered buttons on the coloured banner. The dismiss cross has the same problem, and its scoped red hover rule additionally loses to GButton's more specific transparent-hover rule, so hovering painted a grey box instead of turning the cross red. Give the arrows the transparent blue treatment as props, and raise the dismiss button's scoped selectors above GButton's own.
The published-globe, TRS-shield and URL-import indicators are styled by `.inline-icon-button`, which now loses to GButton's scoped variant rules, so they render as solid grey bordered buttons in the workflow list instead of borderless brand-blue glyphs.
`.inline-icon-button` used to override Bootstrap's `.btn-primary` on the key glyph, marking it out as an indicator next to the solid blue clone, remove and recommend actions. It now loses to GButton's scoped rules and renders as a solid blue box, so state the transparent look as props.
`.btn-transparent-background` only ever targeted `.btn`, so the delete cross on a tag now paints GButton's grey transparent hover box over the tag's own colour. Match `.g-button` too, forcing the hover and active colours past GButton's more specific scoped rules.
`.ui-link` makes a button read as a link, but its single-class `color` and `display` declarations lose to GButton's scoped rules, so both togglers render as grey inline-flex buttons. State the link look as props instead. `transparent color="blue"` alone is not enough: GButton's transparent-blue hover repaints the label near-white for a solid blue fill that `.ui-link` suppresses with `background: none !important`, leaving the toggles invisible on hover. Restate the link colour and underline in a scoped rule specific enough to out-rank it.
`.stateless-tags .toggle-link:hover` loses to GButton's scoped transparent-blue hover rule, so "N more..." and "Fewer tags" paint a solid blue pill; the `padding: 0` rule ties with GButton's size padding and is decided by bundle order. Raise the scoped selector above both.
Both tests dropped to `.vm.$emit("click")` on a GButton stub, which skips
the template's `@click` binding and GButton's own click guard entirely --
they would keep passing with the `@click` deleted. Render the button for
real and trigger a DOM click instead, as ProposalDiffView.test.ts does.
The confirm button no longer renders a native `disabled` attribute, so it stays focusable and keyboard activatable and remains eligible as the form's default button. An implicit form submission bypasses the button's click handler entirely, so check the terms checkbox in `submit()` as well.
BootstrapVue's BLink special-cased an `href` of `#` and swallowed the navigation. GButton has no such guard, so importing a collection also pushed a history entry and scrolled the page to the top. The element is a button already, so the placeholder href can simply go.
9 tasks
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
2 participants
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.
Follow-up to #22688.
That PR converts the last
BButtoncall sites toGButton, and the component-level fixes it needs are already on its own branch. This PR includes the remaining call-site fixes, mostly styling regressions exposed by the conversion, plus three small behavior fixes.All the styling regressions have the same cause. The converted buttons no longer carry
.btn, andGButton's scoped rules (.g-button.g-…[data-v-…], specificity 0,3,0 and up) outrank the global helper classes those call sites still rely on (0,1,0):.inline-icon-button(client/src/style/scss/ui.scss:427),.btn-transparent-background(client/src/style/scss/base.scss:289),.ui-linkand.toggle-link. The visible effect is that brand-blue borderless icons render as grey boxes or bordered chips, and a couple of hover states paint a solid fill where they used to be bare links. Each call site is fixed by passing props that express what the helper used to do (transparent,icon-only,color="blue"), or by raising the scoped selector so it deterministically outranksGButton's own rule, rather than relying on bundle order.Three behavior fixes ride along.
HistoryCounter.test.tsandJobOutputs.test.jshad been weakened to.findComponent(GButton).vm.$emit("click"), which no longer covers the template's@clickwiring orGButton's disabled guard and would pass with the@clickdeleted; both are back to mounting for real and clicking the button.NewUserConfirmationgains anif (!termsRead.value) return;guard insubmit(), becauseGButtonrendersaria-disabledrather than nativedisabled, so an Enter-submit on the form never reaches the button handler and the disabled state alone no longer gates account creation. AndCollectionDisplay's import button washref="#"with a@clickthat had no.prevent; bootstrap-vue'sBLinkspecial-cased a bare#href;GButtondoes not, so importing a collection also pushed a history entry and scrolled the page to the top.One change is worth calling out because the obvious fix would have been wrong.
GCard's indicator buttons neededtransparent color="blue", but hard-coding those props on the element would have silently killedindicator.variantfor callers that set it (useHistoryCardIndicators.ts:75passesvariant: "danger"), because Vue 2'sv-bind="obj"never overwrites an explicitly declared attribute. Instead, the indicator's default variant changed fromoutline-secondarytolink, whichvariantToColoralready maps to{ transparent: true, color: "blue" }, so caller-supplied variants continue to work.Depends on #22688. This branch is stacked on it, so the diff currently includes that PR's component fixes; it will be rebased onto
devonce #22688 merges, which drops them out.How to test the changes?
(Select all options that apply)
cd client && pnpm vitest run src/components/History/CurrentHistory/HistoryCounter.test.ts src/components/JobInformation/JobOutputs.test.jsand confirm both pass — they now mount the component and click the real button rather than emittingclickon a stub./workflows/listwith at least one published workflow and one imported from a TRS server or a URL. The published globe, TRS shield, and URL-import indicators must be borderless brand-blue glyphs, not grey- or white-bordered chips./histories/sharing?id=…). The Edit URL pencil, the Done check, and the Copy button must be borderless blue icons, not full grey bordered buttons./admin/notifications/create_new_broadcast), then view it. The left/right arrows must be borderless, and the dismiss cross must be borderless with a red hover rather than a grey filled square.×on a tag pill: it must stay bare over the tag's coloured background instead of taking a grey hover box.#(no new history entry, no scroll to top).License