Textile sandbox Phlex form #3574
Conversation
- Fixed - updated both integration tests to use textile_sandbox_code as the field ID, which is what Superform generates for a TextileSandbox model with a a code attribute.
- registered asset_path as a value helper using register_value_helper :asset_path, which allows us to call it directly as asset_path("up_arrow.png") instead of view_context.asset_path("up_arrow.png").
- add comment - reformat 3 lines to 1
- Created Components::TextileSandbox - A page component that wraps the form with the help block - Kept Components::TextileSandboxForm - The form itself - Simplified the view - Now it only has the page title and renders the TextileSandbox component
- combined the wrapper and form into a single component. - Render it driectly from the controller
There was a problem hiding this comment.
Pull request overview
This PR refactors the textile sandbox view from ERB to a Phlex form component, following the codebase's ongoing migration to component-based architecture. The textile sandbox allows users to test Textile markup and see rendered output.
- Introduces
TextileSandboxmodel (ActiveModel-backed struct) to encapsulate form data - Creates
Components::TextileSandboxFormPhlex component to replace the ERB view - Updates controller to instantiate the component with proper parameters
- Updates integration and controller tests to use the new form field identifier (
textile_sandbox_code)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
app/models/textile_sandbox.rb |
New ActiveModel struct to hold textile code form data |
app/components/textile_sandbox_form.rb |
New Phlex form component with textile sandbox UI and help links |
app/controllers/info_controller.rb |
Updated to render the Phlex component instead of ERB template |
app/views/controllers/info/textile_sandbox.html.erb |
Deleted ERB view, replaced by Phlex component |
test/integration/capybara/textile_integration_test.rb |
Updated field identifier from "code" to "textile_sandbox_code" |
test/integration/capybara/observations_integration_test.rb |
Updated field identifier from "code" to "textile_sandbox_code" |
test/controllers/info_controller_test.rb |
Changed assertion from assert_template to assert_response(:success) for Phlex compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added rel: "noopener noreferrer" to all four external links with target="_blank" for security.
- Covers previously uncovered lines 58-60 - Implements Copilot suggestion Covers: ✅ Form rendering - textarea field, labels, form action ✅ Initial state - no result section, single submit button ✅ With result state - result section visible, up arrows, both submit buttons ✅ Lines 58-59 coverage - Testing the "show HTML codes" path with render_form_with_result(:sandbox_test_codes.l) which renders the escaped HTML in a <code> tag ✅ Help section - quick reference with HTML entities preserved ✅ External links - All 4 links with proper target="_blank" and rel="noopener noreferrer" attributes ✅ Both submit types - Testing both "Test" and "Test Codes" button behaviors
Fixes brakman warning in prioir commit.
| end | ||
|
|
||
| def view_template | ||
| add_page_title(:sandbox_title.t) |
There was a problem hiding this comment.
TLDR; I think this should be a Phlex view, not a component EDIT component, called from the view.
This is an interesting case because this particular form is only called in this template. (Other forms might be called by both the new and edit actions.)
According to my understanding, a Phlex component should be reusable - it's a swatch of HTML that could be called by two or more templates, ERB or Phlex.
For sure, a component shouldn't be responsible for adding a page title - that's a template's job by definition. A component is page-agnostic.
Phlex templates live in a different place, and they are called by the controller directly, as you have it here, rather than other templates, as components are. Check the Phlex documentation for views. The actual Phlex code you've generated won't need to be much different; it'll just need to live elsewhere.
Come to think of it, i'm not sure all the form components i've generated so far get reused - i will check!
There was a problem hiding this comment.
For a Phlex view, try putting it in the same folder as the ERB view was.
There was a problem hiding this comment.
On third thought — i would propose isolating the form element and anything else that should appear next to it, (sandbox results, help...) and making that the component, in case we ever want to use the textile sandbox in a more versatile way, like say having it appear via Turbo below the form input where someone might need it.
This way we don't have to deal with the question of where to put Phlex view templates yet, which we don't have an answer for.
No description provided.