Repository navigation
GlowBot: carry the 0.2.0 chat memory over, and make the bot page's controls 44 px to tap - #75
Merged
Merged
Conversation
A chat that talked to GlowBot 0.2.0 kept its talk under chat:history. The first time the chat wakes, that talk is put in front of the current one (same limits as any talk), saved, and the old copy is deleted, so /new cannot bring it back. Both keys come in one read, so a chat with nothing to move starts exactly as before.
The header buttons, the small buttons, the Books line, the BotFather link and the token label were 20 to 43 px. Measured in 16 cases (4 states, 320 and 390 px, light and dark): 0 controls under 44 px afterwards, no sideways scroll. Only bot.css changes; the shared Glow header on other pages is untouched. A browser test now measures every control on the live and setup screens.
The 44 px header controls made the header row 8 px taller, which on a 320 px phone slid the Open in Telegram button under the floating assistant button (caught by CI). The controls now grow into the header's own padding with negative block margins: the tap area stays 44 px and the page below sits exactly where it did before.
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.
Two leftovers from the GlowBot work.
1. A chat's 0.2.0 talk is carried over, once
GlowBot 0.2.0 kept each chat's last turns under
chat:history; the unified bot keeps them inside the chat's state, so the old talk was left behind.load()now reads both, adopts the old talk if the chat has none of its own, and deletes the old key. 16 new tests inapps/glow/bot/test/legacy-chat.test.js.2. Every control on the bot page is at least 44 px to tap
Before: 7 to 9 controls under 40 px per screen (header buttons 34 to 36, small buttons 37, the Books line 36.5, the BotFather link 20, the token label 21, the token field 43).
After: 0 under 44 px, measured in 16 cases (ready, setup, unconfigured, offline; 320 and 390 px; light and dark), no sideways scroll.
Only
apps/glow/web/bot/bot.csschanges. The shared Glow header on other pages is not touched.A new browser test,
test_every_control_is_big_enough_for_a_thumb, measures every pressable control on the live and setup screens at 320 and 390 px.Checks
tools/make_ports.py --checkclean.No version bump: ships under the current GlowBot version with a new build hash.
What CI caught, and the fix (b73b0bf)
The first CI run failed one browser test,
test_the_assistant_button_steps_aside_for_what_is_under_it. The 44 px header buttons had made the header row 8 px taller (36 to 44), which on a 320 px phone pushed the Open in Telegram button down until it sat under the floating assistant button. Locally it missed by 0 px, so it only showed with CI's fonts.Fix: the header controls grow into the header's own padding (negative block margins). The tap area stays 44 px, the header row is 36 px again, and the Open in Telegram button sits at exactly the position it has on main in the 6 fonts tried (8 px clear of the assistant button with the default font). The test and the 44 px threshold are unchanged.
After the fix: 0 controls under 44 px in the same 16 cases; the whole browser module passes locally (28 tests; one earlier run under load had a single 10 s wait time out in an unrelated test, which passes alone); CI is green on all five jobs.