-
Notifications
You must be signed in to change notification settings - Fork 894
Mobile: Bridge components #2307
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
Conversation
It sends all buttons in one connect payload.
app/javascript/bridge/controllers/bridge/overflow_menu_controller.js
Outdated
Show resolved
Hide resolved
|
Considering how we decided to move the app related CSS files into the existing structure, should we do the same here? or even flat, like: |
…rget button can be included
... to fix in the native apps
... for template caching
* main: Remove explicit queue definition Fix attribute name Explicitly define incineration priority Fix incorrect recurring job config Add self-service account deletion (#2246) Document DELETE endpoint for removing card header image Document goldness endpoints for marking cards as golden Document closed and column fields in card API response
* main: (22 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: Add rollback script to convert relative URLs back to absolute Add a script to migrate existing rich texts to relative URLs Use relative URLs where possible across all the app Use relative URLs for avatars and rich text attachments
* main: 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 Use correct class selectors to target cards with background images Update lexxy to latest Hide table button for now Table CSS tweaks Update to Lexxy 0.7.0beta Revert "Wrap tables in div" Wrap tables in div CSS updates for `<table>` support Update to latest Lexxy Show only active subscriptions Add paid accounts stats to admin dashboard Keep URLs in webhook events absolute Delete orphaned watches and pins when a card is moved to a private board
... instead of a separate dialog focus controller
... to support lazy loaded dialog content
* main: 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 Add Enable all/Disable all buttons to webhook event selection Remove whitespace Remove hover styles from disabled menu items # Conflicts: # app/views/cards/_container.html.erb
* main: Fix notification broadcast test for turbo-rails 2.0.21 Update `turbo-rails` to get latest Turbo version
kevinmcconnell
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 good! I left one very minor question :)
| if block_given? | ||
| form_with **attributes, data: data, & | ||
| else | ||
| form_with(**attributes, data: data) { } |
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.
I think this makes bridged_form_with behave differently than form_with in the case of no block -- it would make it render an empty form, rather than just an opening form tag. Is this intentional? If not, I think we could do without this conditional, and always pass & to get the matching behaviour.
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.
@svara do you recall why this conditional was added?
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.
I was following the auto_submit_form_with pattern/behaviour.
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.
Ah, I suspect that one has that because it's commonly used as an empty form. So it's probably there as a convenience.
I'd be tempted to remove it from this one, since bridged_form_with is more of a form wrapper, so you'd expect it to behave more like a regular form_with. It's a very minor thing, though!
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.
I appreciate the explanation @kevinmcconnell.
This PR adds Hotwire Native Bridge support and components.
Bridge components:
Example usage:
bridged_form_with.Note: CSS has minimal additions to make the examples work.