feat(PasswordInput)!: build the frame and the toggle, like Number Input - #695
Merged
Conversation
The component already owned the icon; it owns the button and the frame that lays it out now, so the markup is a plain form control: <input type="password" class="form-control" data-coreui-toggle="password-input"> data-coreui-toggle moves from the button to the input - there is no button in the markup to put it on - and its value becomes the component's name, so it reads the same as Number Input's. The wrapping is one helper shared by both components rather than a copy in each: ensureControlGroup / releaseControlGroup in util/form-control-group. It carries the rule Number Input arrived at, that every class except .form-control moves to the frame, because a class on the control describes the field and the field is the frame once it is wrapped. The accessible name of the toggle is an option now (ariaToggleLabel), since the button is no longer the author's to label.
Coverage Report for CI Build 30940890853Coverage increased (+0.02%) to 92.965%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions2 previously-covered lines in 2 files lost coverage.
Coverage Stats💛 - Coveralls |
The Visual job has been passing on every pull request by generating baselines rather than checking against them: with no *-chromium-linux.png in the tree, the matcher creates the set and the run ends green. Visual regressions were only ever caught on a maintainer's machine. These are the 33 the job generated on this branch, so they carry the markup this pull request settles. The next run compares.
The comment said the spinner rules were verified in all three engines. Two were measured; Playwright's WebKit would not launch here, which the pull request body said and the comment did not. It now states what was measured and what each rule leaves behind when used alone.
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.
Number Input landed with the component owning its frame and its buttons, so the markup is a plain form control. Password Input already owned the icon; this gives it the rest.
What changes
data-coreui-togglemoves from the button to the input, and its value becomes the component's name —password-input, matchingnumber-input. There is no button left in the markup to carry it..form-controlmove onto the frame. A class on the control describes the field, and once the input is wrapped the field is the frame —mb-3left behind would put the margin inside the border. They move back ondispose()..form-control-groupis left alone, so a password field can share a frame with other adornments; only a frame the component created is removed ondispose().ariaToggleLabelis a new option: the button is no longer the author's to label.One helper, not two copies
The wrapping is
ensureControlGroup/releaseControlGroupinutil/form-control-group.ts, used by both components. Number Input's inline version is gone — its 20 tests pass against the shared one unchanged, which is the check that mattered for the extraction.The helper carries the rule Number Input arrived at, with the reasoning in one place rather than restated per component.
Verification
data-apiblock is rewritten against the new contract — frame and toggle built on init, clicking toggles bothtypeandaria-pressed, disabled control gets a disabled button, several fields stay independent, classes round-trip throughdispose(), an authored group survives.Bundlewatch budgets nudged for the shared helper; CSS unchanged.