Repository navigation
[SCF-2415] [Part 10] Fix modal footer submission and BsAlert @visible regressions - #15
Merged
Conversation
With push on every branch and pull_request on every base branch, each push to a branch with an open PR ran the full matrix twice. PRs are still covered by the pull_request trigger, which uses '**' so stacked PRs on gaurav/chore/... bases run (b057803). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he type attribute Since f005cd0 the default submit button set its type with @buttonType="submit", which only BsButton understands. A custom @buttonComponent that renders <button type="button" ...attributes> ignored it, so its clicks never submitted the footer form and @onsubmit never ran. Before f005cd0, onClick={{@onsubmit}} reached any button component through ...attributes. Pass type="submit" as an HTML attribute instead. BsButton applies ...attributes after its own type, so it still renders a submit button, and @buttonType is deprecated anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…andles it 6c23b4c made the footer <form> always call preventDefault() on submit, to stop the page reload when its submit button calls @onsubmit. That also blocked native submission where it used to happen: a standalone footer without @onsubmit, and a footer given an action attribute (e.g. <modal.footer action="/export" method="post">). Only prevent the default when @onsubmit is given and the form has no action attribute. That still covers the reload in BsModal, where the footer always gets onSubmit and no action. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8945d07 (upstream ember-bootstrap#2027) replaced `@arg visible = true` with `this._visible ?? true`, so @visible={{null}} (e.g. an unset model attribute) now shows the alert. @arg only falls back to its default for undefined, so null used to hide it. Fall back for undefined only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8945d07 copied @visible into the tracked _visible from a {{did-update this.updateVisibility @visible}} modifier. Writing it during modifier commit schedules a second render, and only that render runs {{did-update this.showOrHide this._visible}}, so each @visible change took an extra render pass before the fade started. Resolve visibility in the visible getter instead. A dismissal records how often @visible had changed at the time (_dismissedAt), and holds until @visible changes again, which the getter counts in untracked fields. That keeps ember-bootstrap#2027's behavior (dismiss doesn't touch @visible, and setting @visible to false and back to true shows the alert again) without writing tracked state while rendering, which is what broke @localcopy on Ember canary. showOrHide's did-update now reads @visible itself, so without a fade onDismissed is called from the afterRender queue instead of from the modifier: a handler that sets @visible (as in ember-bootstrap#2027's test) would otherwise update a value that render has already used. With a fade it was already called later, from a timer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ly under Embroider 7b53aa7 skipped it everywhere for an Embroider-only failure (ember-bootstrap#1682), so classic builds stopped checking that BsForm doesn't swallow errors thrown by @onsubmit. Skip it only when @embroider/core is installed, the check @embroider/test-setup's maybeEmbroider uses to build with Embroider. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 4, 2026
Merged
Merged
Joeu
approved these changes
Nov 4, 2026
Gaurav0
merged commit Nov 5, 2026
ee88a0f
into
gaurav/chore/SCF-2415-part-9-refresh-remaining-deps
152 of 157 checks passed
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
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.
Summary
Fixes for behavior that is already on
scf_master, found while reviewing the SCF-2416 work. They don't depend on any dependency upgrade, so they ship in 4.10.3 rather than waiting for 5.0.0. These are behavior fixes, not vulnerability fixes.@buttonComponentsubmits again. Since f005cd0 the submit button set its type with@buttonType="submit", which onlyBsButtonunderstands. A custom button component ignored it, so@onSubmitnever ran. The footer now passestype="submit"as an HTML attribute instead.@onSubmithandles it. 6c23b4c made the footer<form>always callpreventDefault(), which also blocked a standalone footer without@onSubmitand a footer with anactionattribute. The footer now only prevents the default when@onSubmitis given and the form has noaction.@visible={{null}}hides the alert again. 8945d07 (upstream Refactor bs-alert to remove @localCopy and avoid mutation after consumption error in Ember canary ember-bootstrap/ember-bootstrap#2027) treatednulllikeundefinedand showed the alert.@visiblein the same render. 8945d07 copied@visibleinto tracked state from adid-updatemodifier, so each change took an extra render before the fade started. Visibility is now resolved in the getter, which keeps Refactor bs-alert to remove @localCopy and avoid mutation after consumption error in Ember canary ember-bootstrap/ember-bootstrap#2027's dismiss behavior without writing tracked state during render.scf_masterand tags. Until now, each push to a branch with an open PR ran the full matrix twice. PRs are still covered by thepull_request: '**'trigger from Part 1 (b057803).No dependency or
yarn.lockchanges.Version
No version bump in this part. Part 9 already sets the version to 4.10.3, and
v4.10.3isn't tagged yet, so these fixes ship in 4.10.3. Tag after this part merges.Stack
Part of the SCF-2415 work. This is part 10 of 10. Stacked on Part 9: its base is
gaurav/chore/SCF-2415-part-9-refresh-remaining-deps, so the diff only shows this part. Merge in order.@visibleregressionsThe SCF-2416 stack will be based on this part.
Review notes
Each commit is one fix with its own tests, and each commit message explains the cause. The BsAlert change in "follow BsAlert's @visible in the same render" needs the closest look.
onDismissedwithout a fade now runs from theafterRenderqueue rather than from the modifier.Testing
Run on this branch with Node 18:
yarn install --frozen-lockfilepasses,yarn lintreports 0 errors, andember test(Ember 3.28) has 532 passed, 9 skipped and 0 failed. Part 9 had 526 passed and 10 skipped: this part adds 5 tests and un-skips 1. CI covers the ember-try matrix.🤖 Generated with Claude Code