refactor(web): one name per button, and none painted by hand - #83
Merged
Conversation
Two drifts in opposite directions, both in the templates. vault42-btn-primary and vault42-btn-secondary were pure @apply forwards onto vault42-btn and vault42-btn-outline: two names for one button, so a reader could not tell whether the difference meant anything. Five call sites across two views used them. The call sites now name the thing directly. The @Utility definitions stay in style.css for now, because that file belongs to the stylesheet pass; this is what makes removing them there a deletion rather than a gamble on nobody having typed the old name. MFAOnboardingView went the other way and wrote the palette out longhand -- bg-vault42-primary text-white hover:bg-vault42-primary-hover, which is vault42-btn spelled out, minus the disabled: states it carries. It looked right and would have stopped looking right the first time the utility changed without it. Both buttons now use the shared utilities. The skip link is deliberately left as an anchor. The gate reads templates, so the view somebody adds next is held too. Both arms are mutation-verified: putting either defect back fails it, naming which file and which class. One correction to the gate itself. Its first pattern flagged LanguageSwitcher's dropdown rows, which use bg-vault42-primary/15 to mark the active locale. That is a selection highlight, not a button fill -- \b terminates at the slash, so an opacity modifier read as a bare palette utility. The pattern now excludes it. Coverage is unchanged against main at 99.56 / 98.88 / 100 / 99.88; 750 tests pass; eslint clean.
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.
Two drifts in opposite directions, both in the templates. Phase A items A2 and A4 of the UI plan (#296).
vault42-btn-primaryandvault42-btn-secondarywere pure@applyforwards ontovault42-btnandvault42-btn-outline— two names for one button, so a reader could not tell whether the difference meant anything. Five call sites acrossBlobsViewandIdentityViewused them; they now name the thing directly.The
@utilitydefinitions stay instyle.cssfor now, because that file belongs to the stylesheet pass (B1). This PR is what makes removing them there a deletion rather than a gamble on nobody having typed the old name.MFAOnboardingViewwent the other way and wrote the palette out longhand:That is
vault42-btnspelled out, minus thedisabled:opacity-50 disabled:cursor-not-allowedthe utility carries. It looked right, and would have stopped looking right the first time the utility changed without it. The skip link is deliberately left as an anchor, per the plan's do-not list (#298).The gate reads templates, so the view added next is held too. Both arms are mutation-verified — putting either defect back fails it, naming the file and the class:
One correction to the gate itself, worth noting. Its first pattern flagged
LanguageSwitcher's dropdown rows, which usebg-vault42-primary/15to mark the active locale. That is a selection highlight, not a button fill —\bterminates at the slash, so an opacity modifier read as a bare palette utility. The pattern now excludes it. A gate that flags correct markup is worse than no gate.Verified: 750 web tests pass;
pnpm lintclean; coverage unchanged against main at 99.56 / 98.88 / 100 / 99.88.