-
Notifications
You must be signed in to change notification settings - Fork 908
Mobile/prepare webviews 2 #2309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
... it has the top inset that prevents being under the navigation bar
* main: (73 commits) Fix that caching should be disabled in dev by default. References 3cf841d. Revert "Add CJK (Chinese, Japanese, Korean) search support" Account for new button sizes on mobile Bump filter z-index when filters are open Use a dedicated resource for showing cards in draft mode Refactor popup.css styles for focus state Add Segoe UI Variable Fizzy font face configuration comments Update @layer name to `base` in font-face definition for "Segoe UI Variable Fizzy" Repair scope for card styles within the columns view Add missing list to cards page Account for simpler public views Fix public layout Pull board-tools outside of the list for mobile saas: move log_level setting into an environment variable Restore log level configurability in production environment Remove engagements Go back to board after clearing filters Ensure filters sit on top of cards Fix public boards Fix card grid layout ...
* main: (34 commits) Disable card column buttons when active Prevent comment content from overlapping with reaction button Delete pins belonging to cards in a board with revoked access Update development dependencies in SAAS lockfile Update runtime dependencies Update development dependencies Balance magic link instructions Add `.txt-balance` utility Move handleDesktop inside restoreColumnsDisablingTransitions Adjust border radius Clean up blank slates Add dialog manager to events page More sturated mini bubbles in light mode Brighten mini bubbles in dark mode Use separate cache namespaces for each beta instance Bump boost size up a tick on mobile Move Undo form inside closure message element Phrasing tweak for exceeding storage limit Keep strong tags words on same line Allow ActionText embed reuse and safe purge (#2346) ...
* main: (100 commits) Fix notification broadcast test for turbo-rails 2.0.21 Update `turbo-rails` to get latest Turbo version Remove reference to removed controller Fix bad formatting on bridge page title attr Use private functions for bridge components Setup focus handling on touch target's connect callback Setup proper focus handling for mobile on the card perma's board picker Move dialog focus handling into the dialog controller Create popup initial alignment classes Fix class typo Hide the board selector button if card is closed Align board and tag picker dialogs without using math Add dialog manager to card perma Fix "M" hotkey using stale user from fragment cache Solve problem when Action Text raises on missing attachables Solve problem when Action Text raises on missing attachables Completing a step removes stalled status from UI Bump Bootsnap to v1.21.1 Add Enable all/Disable all buttons to webhook event selection Use correct class selectors to target cards with background images ...
* main: Return to Board page when clearing filters Bump card perma z-index when dialog is open Only submit on blur if the input has a value Make the CLAUDE.md stub less obtrusive Only index up to 32KB of search content Use inline spacing variable Actually make it affect larger screens too Use CSS variable for panel size in delete dialogs Add viewport padding to dialogs on mobile Prevent page scrolling when modal dialog is open
jayohms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @adjogima 👍
... fixes content being stuck behind the tab bar in apps
andyra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good! Just a couple suggestions around formatting.
|
|
||
| :root { | ||
| /* Insets - The mobile apps may inject their own custom insets based on native elements on screen, like a floating navigation */ | ||
| --custom-safe-inset-top: var(--injected-safe-inset-top, env(safe-area-inset-top, 0px)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need both --custom-safe-inset-<side> and --injected-safe-inset-<side>? Kinda seems like we could just use the env() value as a default, then override it with a custom value when required. Something like this?
:root {
--safe-inset-top: env(save-area-inset-top, 0px);
}
/* Uses the default value */
.header {
margin-block-start: var(--safe-inset-top);
}
/* Uses a custom value */
.header.is-special {
--safe-inset-top: 3rem;
}There also may be some finer points about how the custom props are injected, so there may well be a good reason for having two separate custom props. Anyway, a minor detail—looks good otherwise!
| el.classList.add("orient-left") | ||
| } else if (leftSpace < EDGE_THRESHOLD && leftSpace < rightSpace) { | ||
| el.classList.add("orient-right") | ||
| if (targetGeometry.spaceOnRight < EDGE_THRESHOLD && targetGeometry.spaceOnRight < targetGeometry.spaceOnLeft) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be helpful to split these out into named variables / functions to make it a little more clear what the intent is. Something like this, perhaps?
const shouldOrientLeft = targetGeometry.spaceOnRight < EDGE_THRESHOLD && targetGeometry.spaceOnRight < targetGeometry.spaceOnLeft
const shouldOrientRight = targetGeometry.spaceOnLeft < EDGE_THRESHOLD && targetGeometry.spaceOnLeft < targetGeometry.spaceOnRight
if (shouldOrientLeft) {
this.#orientLeft()
} else if (shouldOrientRight) {
this.#orientRight()
}
function orientLeft() {
// offset math, classList, and CSS props set here
}
function orientRight() {
// offset math, classList, and CSS props set here
}| @@ -1,7 +1,25 @@ | |||
| @layer platform { | |||
| :root[data-text-size=xsmall]:has([data-platform~=ios]) { font-size: 14px; } | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a little cleaner to nest this to remove some of the repetition:
:root:has([data-platform="ios"]) {
&[data-text-size=xsmall] { font-size: 14px; }
&[data-text-size=small] { font-size: 15px; }
&[data-text-size=medium] { font-size: 16px; }
&[data-text-size=large] { font-size: 17px; }
&[data-text-size=xlarge] { font-size: 19px; }
&[data-text-size=xxlarge] { font-size: 21px; }
&[data-text-size=xxxlarge] { font-size: 23px; }
}
Refining the webviews for mobile web and mobile apps: