max / alloy
1 file changed,
+61 insertions,
-37 deletions
| @@ -1851,15 +1851,10 @@ | |||
| 1851 | 1851 | match key.code { | |
| 1852 | 1852 | KeyCode::Tab | KeyCode::Down => self.machine.next(), | |
| 1853 | 1853 | KeyCode::BackTab | KeyCode::Up => self.machine.prev(), | |
| 1854 | - | // Enter advances off the field and submits from the checkbox, which | |
| 1855 | - | // is the shape the account step already uses: on both screens the | |
| 1856 | - | // last slot is the one that commits. | |
| 1857 | - | KeyCode::Enter => { | |
| 1858 | - | if self.machine.current() == SLOT_TIMEZONE { | |
| 1859 | - | return self.name_machine(); | |
| 1860 | - | } | |
| 1861 | - | self.machine.next(); | |
| 1862 | - | } | |
| 1854 | + | // Enter submits from either slot, the same shape the account step | |
| 1855 | + | // uses. The checkbox has a default, so committing from the name | |
| 1856 | + | // field is a complete answer rather than a half-filled one. | |
| 1857 | + | KeyCode::Enter => return self.name_machine(), | |
| 1863 | 1858 | KeyCode::Char(' ') if self.machine.current() == SLOT_TIMEZONE => { | |
| 1864 | 1859 | self.locate_timezone = !self.locate_timezone; | |
| 1865 | 1860 | } | |
| @@ -1926,19 +1921,17 @@ | |||
| 1926 | 1921 | ||
| 1927 | 1922 | /// Keys for the account step's three fields. | |
| 1928 | 1923 | /// | |
| 1929 | - | /// Enter advances the focus ring rather than submitting, until the last | |
| 1930 | - | /// field. Submitting from the middle of a form is a way to have the second | |
| 1931 | - | /// half silently empty. | |
| 1924 | + | /// Enter submits from whichever field has focus, which is what the footer | |
| 1925 | + | /// has always promised. Moving between fields is Tab's job. Submitting from | |
| 1926 | + | /// the middle cannot leave the second half empty: [`create_account`] | |
| 1927 | + | /// validates every field and focuses the first one that refuses. | |
| 1928 | + | /// | |
| 1929 | + | /// [`create_account`]: Self::create_account | |
| 1932 | 1930 | fn edit_account(&mut self, key: KeyEvent) -> Flow { | |
| 1933 | 1931 | match key.code { | |
| 1934 | 1932 | KeyCode::Tab | KeyCode::Down => self.fields.next(), | |
| 1935 | 1933 | KeyCode::BackTab | KeyCode::Up => self.fields.prev(), | |
| 1936 | - | KeyCode::Enter => { | |
| 1937 | - | if self.fields.current() == FIELD_PUBKEY { | |
| 1938 | - | return self.create_account(); | |
| 1939 | - | } | |
| 1940 | - | self.fields.next(); | |
| 1941 | - | } | |
| 1934 | + | KeyCode::Enter => return self.create_account(), | |
| 1942 | 1935 | KeyCode::Char(c) => self.focused_field().insert(c), | |
| 1943 | 1936 | KeyCode::Backspace => self.focused_field().backspace(), | |
| 1944 | 1937 | KeyCode::Delete => self.focused_field().delete(), | |
| @@ -2297,7 +2290,7 @@ | |||
| 2297 | 2290 | } | |
| 2298 | 2291 | name.push(text::primary(theme, after.to_string())); | |
| 2299 | 2292 | ||
| 2300 | - | let ticked = self.machine.is_focused(SLOT_TIMEZONE); | |
| 2293 | + | let boxed = self.machine.is_focused(SLOT_TIMEZONE); | |
| 2301 | 2294 | let box_glyph = if self.locate_timezone { "[x]" } else { "[ ]" }; | |
| 2302 | 2295 | let label = "set timezone from my location"; | |
| 2303 | 2296 | ||
| @@ -2306,16 +2299,31 @@ | |||
| 2306 | 2299 | Line::default(), | |
| 2307 | 2300 | Line::from(name), | |
| 2308 | 2301 | Line::default(), | |
| 2302 | + | // Focus is drawn the same way it is on the field above: the box | |
| 2303 | + | // takes the REVERSED block the caret uses, so one vocabulary says | |
| 2304 | + | // "here" on both slots. Weight alone did not carry it — bold and | |
| 2305 | + | // plain at the same foreground color are near enough identical on | |
| 2306 | + | // most terminal themes to read as no change at all. | |
| 2307 | + | // | |
| 2308 | + | // The label dims with the box rather than staying at primary. | |
| 2309 | + | // Everything else on this pane goes muted when it loses focus, and | |
| 2310 | + | // a label that stayed bright while the hostname above it dimmed | |
| 2311 | + | // made the pair look inverted. | |
| 2309 | 2312 | Line::from(vec![ | |
| 2310 | - | if ticked { | |
| 2311 | - | text::bold(theme, format!("{box_glyph} ")) | |
| 2313 | + | if boxed { | |
| 2314 | + | Span::styled( | |
| 2315 | + | format!("{box_glyph} "), | |
| 2316 | + | Style::default() | |
| 2317 | + | .fg(theme.content_primary) | |
| 2318 | + | .add_modifier(Modifier::REVERSED), | |
| 2319 | + | ) | |
| 2312 | 2320 | } else { | |
| 2313 | 2321 | text::muted(theme, format!("{box_glyph} ")) | |
| 2314 | 2322 | }, | |
| 2315 | - | if ticked { | |
| 2323 | + | if boxed { | |
| 2316 | 2324 | text::bold(theme, label) | |
| 2317 | 2325 | } else { | |
| 2318 | - | text::primary(theme, label) | |
| 2326 | + | text::muted(theme, label) | |
| 2319 | 2327 | }, | |
| 2320 | 2328 | ]), | |
| 2321 | 2329 | ]; | |
| @@ -2883,7 +2891,6 @@ | |||
| 2883 | 2891 | } | |
| 2884 | 2892 | type_into(&mut view, "workshop", &mut log); | |
| 2885 | 2893 | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 2886 | - | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 2887 | 2894 | ||
| 2888 | 2895 | assert_eq!(view.answers.hostname.as_deref(), Some("workshop")); | |
| 2889 | 2896 | assert!(view.error.is_none()); | |
| @@ -2923,7 +2930,6 @@ | |||
| 2923 | 2930 | let (mut view, mut log) = at_hostname(); | |
| 2924 | 2931 | type_into(&mut view, "_", &mut log); | |
| 2925 | 2932 | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 2926 | - | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 2927 | 2933 | ||
| 2928 | 2934 | assert_eq!(view.step(), Step::Hostname); | |
| 2929 | 2935 | assert_eq!(view.answers.hostname, None); | |
| @@ -2999,10 +3005,8 @@ | |||
| 2999 | 3005 | ||
| 3000 | 3006 | fn at_account() -> (InstallView, CommandLog) { | |
| 3001 | 3007 | let (mut view, mut log) = at_hostname(); | |
| 3002 | - | // Twice: the step has two slots now, and Enter walks off the field | |
| 3003 | - | // before it submits. The checkbox is left alone, so this arrives at the | |
| 3004 | - | // account with the timezone answer at its default. | |
| 3005 | - | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 3008 | + | // Once: Enter submits from either slot. The checkbox is left alone, so | |
| 3009 | + | // this arrives at the account with the timezone answer at its default. | |
| 3006 | 3010 | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 3007 | 3011 | assert_eq!(view.step(), Step::Account, "stalled on the hostname"); | |
| 3008 | 3012 | (view, log) | |
| @@ -3010,9 +3014,9 @@ | |||
| 3010 | 3014 | ||
| 3011 | 3015 | /// Fill the account fields, moving between them the way a user does. | |
| 3012 | 3016 | /// | |
| 3013 | - | /// Leaves focus on the key field with it empty, which is where Enter | |
| 3014 | - | /// submits. That is also the default install: the key is optional, so a user | |
| 3015 | - | /// who does not want one tabs past it. | |
| 3017 | + | /// Leaves focus on the key field with it empty. That is the default | |
| 3018 | + | /// install: the key is optional, so a user who does not want one tabs past | |
| 3019 | + | /// it. | |
| 3016 | 3020 | fn fill_account(view: &mut InstallView, user: &str, pass: &str, confirm: &str) { | |
| 3017 | 3021 | let mut log = CommandLog::new(); | |
| 3018 | 3022 | view.fields.focus(FIELD_USERNAME); | |
| @@ -3048,16 +3052,36 @@ | |||
| 3048 | 3052 | assert_eq!(view.confirm.value(), ""); | |
| 3049 | 3053 | } | |
| 3050 | 3054 | ||
| 3051 | - | // Enter in the middle of a form advances the focus rather than submitting. | |
| 3052 | - | // Submitting from the first field is how the second half ends up empty. | |
| 3055 | + | // Enter submits from wherever it is pressed. Walking the fields is Tab's | |
| 3056 | + | // job, and the footer says so. What stops a half-filled account from | |
| 3057 | + | // getting through is validation, not the position of the caret: submitting | |
| 3058 | + | // with only a username lands back on the step with the empty password | |
| 3059 | + | // named, rather than advancing. | |
| 3053 | 3060 | #[test] | |
| 3054 | - | fn enter_walks_the_fields_before_it_submits() { | |
| 3061 | + | fn enter_submits_from_the_first_field_and_validation_refuses_it() { | |
| 3055 | 3062 | let (mut view, mut log) = at_account(); | |
| 3056 | 3063 | type_into(&mut view, "max", &mut log); | |
| 3057 | 3064 | ||
| 3058 | 3065 | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 3059 | - | assert_eq!(view.step(), Step::Account, "submitted from the first field"); | |
| 3060 | - | assert_eq!(view.fields.current(), FIELD_PASSWORD); | |
| 3066 | + | ||
| 3067 | + | assert_eq!(view.step(), Step::Account, "advanced without a password"); | |
| 3068 | + | assert_eq!(view.answers.username, None); | |
| 3069 | + | assert_eq!(view.fields.current(), FIELD_CONFIRM); | |
| 3070 | + | assert!(view.error.unwrap().contains("password is required")); | |
| 3071 | + | } | |
| 3072 | + | ||
| 3073 | + | // The other half of the same rule: a complete account submits from the | |
| 3074 | + | // first field too, without tabbing to the end of the form first. | |
| 3075 | + | #[test] | |
| 3076 | + | fn a_complete_account_submits_from_any_field() { | |
| 3077 | + | let (mut view, mut log) = at_account(); | |
| 3078 | + | fill_account(&mut view, "max", "hunter2", "hunter2"); | |
| 3079 | + | view.fields.focus(FIELD_USERNAME); | |
| 3080 | + | ||
| 3081 | + | view.handle(KeyEvent::from(KeyCode::Enter), &mut log); | |
| 3082 | + | ||
| 3083 | + | assert_eq!(view.step(), Step::Summary); | |
| 3084 | + | assert_eq!(view.answers.username.as_deref(), Some("max")); | |
| 3061 | 3085 | } | |
| 3062 | 3086 | ||
| 3063 | 3087 | // The mistake this step exists to catch. A typo in a value nobody can see |