Skip to content

[chore] add forms dsl input for filterable tree views - #23533

Merged
Kharonus merged 5 commits into
devfrom
chore/forms-dsl-input-for-filterable-tree-view
Jun 12, 2026
Merged

[chore] add forms dsl input for filterable tree views#23533
Kharonus merged 5 commits into
devfrom
chore/forms-dsl-input-for-filterable-tree-view

Conversation

@Kharonus

@Kharonus Kharonus commented Jun 3, 2026

Copy link
Copy Markdown
Member

What are you trying to accomplish?

  • get a forms dsl input filterable_tree_view

What approach did you choose and why?

  • pass every input argument unchanged to filterable tree view component
  • no default logic
  • label defaults to nil

@Kharonus
Kharonus requested a review from myabc June 3, 2026 10:55
@Kharonus Kharonus self-assigned this Jun 3, 2026
@Kharonus
Kharonus force-pushed the chore/forms-dsl-input-for-filterable-tree-view branch from a004986 to ddae922 Compare June 3, 2026 10:56
@Kharonus

Kharonus commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Ahoi @myabc ,

you asked or adding the filterable tree view as a forms dsl input. Can you check my code here? I'm not very used to those components, do I miss something obvious? For my use case this works quite fine. Is the simple approach of not applying any defaults correct?

Base automatically changed from implementation/73350-add-search-wiki-page-dialog to dev June 3, 2026 13:03

@myabc myabc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Kharonus yes, this looks good! thanks for doing rolling this into an input.

A few small asks:

  • a spec for the input. This recently-added spec should be a good template: spec/lib/primer/open_project/forms/segmented_control_spec.rb.
  • add a filterable_tree_view example to spec/lib/primer/open_project/forms/dsl/input_methods_spec.rb as well - admittedly this spec is a bit stupid but it exists to verify that attribute help text options are correctly handled.

@myabc

myabc commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

This probably might need a bit more thought after all - I'll try to discuss with @HDinger tomorrow.

In your particular case you don't need to render a label, which is fine. However most "inputs" support a label - and for symmetry we should support it here.

This might mean:

  • forwarding label: and visually_hide_label: to filter_input_arguments:
  • or alternatively, wrap in a fieldset, with the label being a rendered as a legend.

Either way this brings up an interesting discussion of what the "input" actually consists of in this particular case!

@Kharonus

Kharonus commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

This probably might need a bit more thought after all - I'll try to discuss with @HDinger tomorrow.

In your particular case you don't need to render a label, which is fine. However most "inputs" support a label - and for symmetry we should support it here.

This might mean:

  • forwarding label: and visually_hide_label: to filter_input_arguments:
  • or alternatively, wrap in a fieldset, with the label being a rendered as a legend.

Either way this brings up an interesting discussion of what the "input" actually consists of in this particular case!

Yes, I suspected as much, when I saw that label is actually mandatory, but the filterable tree view doesn't have one. So I set it to nil in the initialize.

Please tell me, when you discussed a solution.

//cc @HDinger @myabc

<%= @input.label %>
<% end %>
<% end %>
<div class="mb-2">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@HDinger I chose to always render the bottom margin - as (GH Primer, non-advanced) check box group does.

@myabc
myabc force-pushed the chore/forms-dsl-input-for-filterable-tree-view branch from 7e2782c to 64dea7b Compare June 11, 2026 13:14
Comment thread modules/wikis/app/forms/wikis/link_existing_wiki_page_form.rb
@myabc
myabc requested a review from HDinger June 11, 2026 13:28
Kharonus and others added 3 commits June 11, 2026 14:29
Fieldset-style inputs never go through the `FormControl` wrapper, so the
legend needs the `FormControl-label` class applied by the component. The
input does not support validation for the time being.

Wrapper arguments stay on the fieldset, while the component-specific and
form arguments flow to the inner tree view.
@myabc
myabc force-pushed the chore/forms-dsl-input-for-filterable-tree-view branch from a9e200b to 33cdb92 Compare June 11, 2026 13:33
myabc added 2 commits June 11, 2026 14:38
Covers the hidden link attributes and the wiki page selection tree.
Enables the shared help-text examples for the new tree view input.
@myabc
myabc force-pushed the chore/forms-dsl-input-for-filterable-tree-view branch from 33cdb92 to ae83523 Compare June 11, 2026 13:38
@myabc

myabc commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@HDinger should we add documentation for this input to the Lookbook in this PR? Or take care of it as a follow-up?

@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/projects/lists/filters_spec.rb[1:6:1]

@Kharonus

Copy link
Copy Markdown
Member Author

tested, looks good for me.

@Kharonus
Kharonus merged commit 1078440 into dev Jun 12, 2026
16 checks passed
@Kharonus
Kharonus deleted the chore/forms-dsl-input-for-filterable-tree-view branch June 12, 2026 08:35
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants