Skip to content

Conversation

hectorgarcia-cw
Copy link

Summary

This PR adds support for displaying ui_button inputs in a modal dialog instead of the traditional dropdown by adding a new show_inputs_as_modal parameter to v1alpha1.ui_button().

Fixes

Fixes #5907

Changes Made

Backend Changes

  • Added ShowInputsAsModal field to pkg/apis/core/v1alpha1/uibutton_types.go

    • New boolean field with proper protobuf annotations
    • Includes documentation explaining the feature
  • Updated Starlark API in internal/tiltfile/v1alpha1/types.go

    • Added show_inputs_as_modal parameter to ui_button() function
    • Maintains backward compatibility (defaults to false)

Frontend Changes

  • New Modal Component (web/src/ApiButtonInputModal.tsx)

    • Material-UI Dialog implementation
    • Handles form submission and cancellation
    • Proper accessibility support with focus management
  • Updated ApiButton Logic (web/src/ApiButton.tsx)

    • Detects showInputsAsModal flag and displays modal instead of dropdown
    • Maintains existing dropdown behavior when flag is false or unset
  • Comprehensive Tests (web/src/ApiButtonInputModal.test.tsx)

    • Tests modal rendering, input handling, and user interactions
    • Ensures proper form validation and accessibility

Auto-Generated Files

  • Updated pkg/apis/core/v1alpha1/generated.proto (via make update-codegen-go)
  • Updated Starlark bindings (via make update-codegen-starlark)

Usage Example

# Traditional dropdown (existing behavior)
v1alpha1.ui_button(
  'deploy_btn', 
  text='Deploy',
  inputs=[v1alpha1.ui_input_spec(name='env', text=v1alpha1.ui_text_input_spec())]
)

# New modal popup
v1alpha1.ui_button(
  'deploy_modal_btn', 
  text='Deploy with Modal',
  inputs=[v1alpha1.ui_input_spec(name='env', text=v1alpha1.ui_text_input_spec())],
  show_inputs_as_modal=True  # ✨ New parameter
)

@hectorgarcia-cw
Copy link
Author

Test Tiltfile

# Dropdown vs Modal Comparison Demo
# This Tiltfile demonstrates the same ui_button functionality in both modes

# Sample resource to host the buttons
local_resource('demo', 'echo "Dropdown vs Modal comparison demo"')

# Common inputs for both button types
deploy_inputs = [
    v1alpha1.ui_input_spec(
        name='environment',
        label='Environment',
        text=v1alpha1.ui_text_input_spec(default_value='staging')
    ),
    v1alpha1.ui_input_spec(
        name='enable_debug',
        label='Enable Debug Mode',
        bool=v1alpha1.ui_bool_input_spec(default_value=False)
    )
]

# TRADITIONAL DROPDOWN BUTTON (existing behavior)
v1alpha1.ui_button(
    'deploy_dropdown',
    text='Deploy (Dropdown)',
    location={
        'component_type': 'resource',
        'component_id': 'demo'
    },
    inputs=deploy_inputs,
    icon_name='play_arrow'
)

# NEW MODAL BUTTON (new behavior)
v1alpha1.ui_button(
    'deploy_modal',
    text='Deploy (Modal)',
    location={
        'component_type': 'resource',
        'component_id': 'demo'
    },
    inputs=deploy_inputs,
    show_inputs_as_modal=True,   # New modal popup
    icon_name='launch'
)

print("Comparison demo loaded!")
print("Try both buttons to see the difference:")
print("  - Dropdown: shows inputs in floating panel")
print("  - Modal: shows inputs in centered dialog")

@nicks
Copy link
Member

nicks commented Oct 13, 2025

hello! thanks for the contribution!

i have a high-level question -- should we just make the new dialog the default behavior? i'm not sure i see a strong case for making it an option if we think the dialog is better.

@hectorgarcia-cw
Copy link
Author

My motivation in doing like this is because I'm a big fan of keeping backwards compatibility.
To make it default I would introduce it, leave it sit a couple of versions and change default behaviour after some notice.
But, I'm willing to make it default rigth away if that is what is wanted.

@hectorgarcia-cw
Copy link
Author

I just forced push to fix my email and sign-off the commit message

@nicks nicks self-requested a review October 14, 2025 13:26
@nicks
Copy link
Member

nicks commented Oct 20, 2025

hmmm....this doesn't seem to be implemented right.

in the original ticket, they asked for a modal to replace the existing dropdown menu.

That's not what i'm seeing in this PR -- I'm seeing:

  • the dropdown menu is still there
  • if i click on the main button (not the dropdown), i get a modal

so it's actually implementing both UIs in one component?

2025-10-20T15:50:23,709830900-04:00 2025-10-20T15:50:18,118749920-04:00

@nicks
Copy link
Member

nicks commented Oct 20, 2025

let's get rid of show_inputs_as_modal.

if we think the modal is the better UI, let's just replace the dropdown -- i don't see a good reason to make it an option

@hectorgarcia-cw
Copy link
Author

Ok. Done. Replaced dropdown with modal. Much simpler PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Button should allows to show inputs on a modal popup

2 participants