Skip to content

wip#1548

Draft
jjaffeux wants to merge 168 commits intomainfrom
d-sheets
Draft

wip#1548
jjaffeux wants to merge 168 commits intomainfrom
d-sheets

Conversation

@jjaffeux
Copy link
Owner

No description provided.

@jjaffeux
Copy link
Owner Author

@greptile review

@greptile-apps
Copy link

greptile-apps bot commented Jan 26, 2026

Greptile Summary

Implemented a comprehensive bottom sheet/modal system (d-sheet) for float-kit with extensive features including:

  • Core Components: Added 2169-line controller managing sheet lifecycle, animations, and interactions through hierarchical state machines
  • State Management: Implemented robust state machine system (1099 lines) with Glimmer tracking for reactive updates and guarded transitions
  • Services: Created sheet-registry, sheet-stack-registry, and theme-color-manager services for centralized sheet management, scroll locking, and inert element handling
  • Scroll Components: Built d-scroll component suite for gesture-based scrolling with focus management, safe area handling, and keyboard interactions
  • Platform Support: Extended capabilities service with browser engine detection (isChromium, isWebKit, browserEngine) and platform-specific capabilities (isAppleMobile, detectedPlatform, isAndroidChromiumBrowser)
  • Utilities: Added keyboard visibility detection and scroll behavior helpers to utilities
  • Toast Improvements: Enhanced toast service with proper null coalescing (options.data ??= {}) to prevent undefined property access, added height tracking and stack ordering
  • Plugin Integration: Updated discourse-assign plugin to use new sheet-based UI with nested stacks

The implementation includes extensive CSS (948 lines for d-sheet), comprehensive touch handling, animation systems, dimension calculations, and accessibility features. The PR title "wip" suggests this is work in progress.

Confidence Score: 3/5

  • This PR is a work-in-progress with significant implementation but contains commented-out code that should be cleaned up
  • Score reflects the WIP nature of the PR (indicated by title), the massive scope (18K+ lines), and presence of commented-out code in the plugin initializer. The core implementation appears solid with proper null safety and comprehensive architecture, but the large surface area and WIP status warrant caution before merging.
  • Pay attention to plugins/discourse-assign/assets/javascripts/discourse/initializers/extend-for-assigns.js which has 50+ lines of commented code that should be removed

Important Files Changed

Filename Overview
frontend/discourse/float-kit/components/d-sheet/controller.js Added 2169-line controller managing sheet lifecycle, animations, and interactions using state machines
frontend/discourse/float-kit/components/d-sheet/state-machine.js Implemented hierarchical state machine with Glimmer tracking for sheet state management (1099 lines)
plugins/discourse-assign/assets/javascripts/discourse/initializers/extend-for-assigns.js Has large block of commented-out code (registerTopicFooterButton) that should be removed
frontend/discourse/float-kit/services/sheet-registry.js Created service for managing sheet instances, scroll lock, and centralized inert management (669 lines)

Sequence Diagram

sequenceDiagram
    participant User
    participant Trigger as DSheet.Trigger
    participant Root as DSheet.Root
    participant Controller
    participant StateMachine
    participant Registry as SheetRegistry
    participant View as DSheet.View
    participant Animation
    participant Touch as TouchHandler
    
    User->>Trigger: Click trigger button
    Trigger->>Root: present()
    Root->>Root: openSheet()
    Root->>Controller: open()
    
    Controller->>StateMachine: send("OPEN")
    StateMachine->>StateMachine: Transition to "opening"
    StateMachine->>Controller: Notify state change
    
    Controller->>Registry: register(controller)
    Registry->>Registry: applyScrollLock()
    Registry->>Registry: recalculateInertOutside()
    
    Controller->>View: Mount view with isPresented=true
    View->>Controller: Register DOM elements
    Controller->>Controller: Calculate dimensions
    Controller->>Animation: Start opening animation
    
    Animation->>View: Apply CSS transforms
    Animation-->>StateMachine: Animation complete
    StateMachine->>StateMachine: Transition to "open"
    
    User->>View: Swipe gesture
    View->>Touch: handleScrollStart()
    Touch->>Controller: onTouchGestureStart()
    Controller->>StateMachine: Update touch state
    
    User->>View: Release swipe
    View->>Touch: handleScrollEnd()
    Touch->>Controller: onTouchGestureEnd()
    Touch->>Touch: startScrollendMonitor()
    
    alt Swiped to dismiss
        Touch->>Controller: close()
        Controller->>StateMachine: send("ACTUALLY_CLOSE")
        StateMachine->>StateMachine: Transition to "closing"
        Controller->>Animation: Start closing animation
        Animation->>View: Apply exit transforms
        Animation-->>StateMachine: Animation complete
        StateMachine->>StateMachine: Transition to "closed"
        Controller->>Registry: unregister(controller)
        Registry->>Registry: removeScrollLock()
        Controller->>Root: onClosed callback
    else Stayed open
        Touch->>Touch: Snap back to position
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (5)

  1. frontend/discourse/float-kit/services/toasts.js, line 68 (link)

    logic: options.data will throw TypeError if options.data is undefined

  2. frontend/discourse/float-kit/services/toasts.js, line 82-83 (link)

    logic: options.data will throw TypeError if options.data is undefined

  3. frontend/discourse/float-kit/services/toasts.js, line 97-98 (link)

    logic: options.data will throw TypeError if options.data is undefined

  4. frontend/discourse/float-kit/services/toasts.js, line 112-113 (link)

    logic: options.data will throw TypeError if options.data is undefined

  5. frontend/discourse/float-kit/services/toasts.js, line 127-128 (link)

    logic: options.data will throw TypeError if options.data is undefined

108 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

@jjaffeux
Copy link
Owner Author

@greptile review

@greptile-apps
Copy link

greptile-apps bot commented Jan 26, 2026

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

xfalcox and others added 30 commits February 27, 2026 10:25
Bump discourse_ai-tokenizers from 0.4.1 to 0.4.2.
discourse#38112)

The sidebar "New Topic" button in Horizon was passing `this.tag?.id` (a
numeric ID) to `composer.openNewTopic()`, which expects a
comma-separated string of tag names. This caused two issues:

- For non-staff users, `filterTags()` tried to access `.content` on a
number, throwing "Cannot read properties of undefined (reading
'filter')" and preventing the composer from opening entirely.
- For staff users, `filterTags()` passed the number through, resulting
in the numeric ID being displayed as the tag name in the composer.

Switching to `this.tag?.name` aligns with the pattern used by the
discovery list controller and the composer's expected API.

Also adds a system spec covering the tag page → new topic flow.

Ref - t/178629
An admin could set `pg_function` to an arbitrary SQL fragment that would be string-interpolated into raw SQL queries in`DiscourseAi::Embeddings::Schema`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment