Say what the login form layout does, not what it replaced - #41
Merged
Merged
Conversation
Follows-up to #40 Three comments and a test name described the login form as it was before that pull request rather than as it is: the watermark "led the form while nothing was described below zero", core's button had to be demoted "now that" the form recommends the member route, and a test was named for that button no longer being primary. A reader of master cannot check any of it, and the mechanism that makes the code necessary — an unweighted field sinks under a section weighted below zero — was left implied. Each now states that mechanism in the present tense. Two smaller ones. `PASSWORD_FIELD` and `PASSWORD_LOGIN_BUTTON` shared one gloss though they answer different questions, and they disagree on a wiki whose only other route is single sign-on, where the divider is suppressed while the demotion still fires. And `LoginCodeRequest` asserts that the form's first button names no route without pointing at what makes that true. Comments only: the code tokens of both source files are unchanged and the stylesheet is untouched, so the rendered form cannot differ. Considered, omitted: * Deleting `testDefaultButtonIsLeftOutOfTheTabOrder`, which no mutation reddens on its own. It names a promise a reader would look for, and dropping a test to save three lines is a poor trade. * Folding the `mfLogo` re-weight into `CAPTCHA_WEIGHTS`, or a shared helper for the two: two call sites with different guards and different reasons. * Reusing `fallbackLine()` for the hidden submit field: would add a parameter for one caller and rename the concept. * Renaming `testMobileWatermarkGoesOnLeadingTheForm`: "goes on" reads as the invariant the handler preserves, matching `testCollectedFieldStaysBelowTheButtonThatEntersTheCode` beside it. * The five negative weight constants, the single-assignment private method, and the field-name constants: each follows a convention the class already had. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41 +/- ##
=========================================
Coverage 87.71% 87.71%
Complexity 634 634
=========================================
Files 71 71
Lines 2239 2239
=========================================
Hits 1964 1964
Misses 275 275 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Follows-up to #40
Three comments and a test name described the login form as it was before that pull request rather
than as it is: the watermark "led the form while nothing was described below zero", core's button
had to be demoted "now that" the form recommends the member route, and a test was named for that
button no longer being primary. A reader of master cannot check any of it, and the mechanism that
makes the code necessary — an unweighted field sinks under a section weighted below zero — was left
implied. Each now states that mechanism in the present tense.
Two smaller ones.
PASSWORD_FIELDandPASSWORD_LOGIN_BUTTONshared one gloss though they answerdifferent questions, and they disagree on a wiki whose only other route is single sign-on, where the
divider is suppressed while the demotion still fires. And
LoginCodeRequestasserts that the form'sfirst button names no route without pointing at what makes that true.
Comments only: the code tokens of both source files are unchanged and the stylesheet is untouched,
so the rendered form cannot differ.
Considered, omitted:
testDefaultButtonIsLeftOutOfTheTabOrder, which no mutation reddens on its own. It namesa promise a reader would look for, and dropping a test to save three lines is a poor trade.
mfLogore-weight intoCAPTCHA_WEIGHTS, or a shared helper for the two: two callsites with different guards and different reasons.
fallbackLine()for the hidden submit field: would add a parameter for one caller andrename the concept.
testMobileWatermarkGoesOnLeadingTheForm: "goes on" reads as the invariant the handlerpreserves, matching
testCollectedFieldStaysBelowTheButtonThatEntersTheCodebeside it.constants: each follows a convention the class already had.