mathesar icon indicating copy to clipboard operation
mathesar copied to clipboard

Improve focus indicator of `InputGroup` component

Open pavish opened this issue 2 years ago • 2 comments

The form-builder PR introduced the InputGroup component, which helped make the input components more cleaner but lead to a stylistic regression.

Comments from @seancolsen on the PR:

I really like the changes that simplify TextInput to a single DOM element, and I want to make sure to keep those changes. There's also clearly value in keeping the "grouped input" functionality. But the changes in this PR lead to a minor stylistic regressions in my opinion.

Examples:

  • Filter tables

    Before After
    image image
  • Add column

    Before After
    image image

I'd prefer that the focus indicator encompass the entire group, like it did before.

I'd like to keep the changes that add a grey background to the group items as it helps to distinguish the label from the input.

In Storybook, I see that InputGroup is also intended to handle the use case of multiple inputs grouped together, in which case the focus indicators would need to be separate. But this use-case seems quite esoteric to me. I can't recall ever having seen a UI like that. I'd suggest we abandon any attempts to support it.

Proposed solution

  1. We need to analyze if it's worth it to bind focus indicator between the InputGroup component and the slotted input components.
  2. Bootstrap's InputGroup was the main inspiration for this component. We can further analyze the need to group multiple inputs together instead of just text with input.

Based on the analysis, we'd like to perform necessary improvements on the component.

pavish avatar Jan 11 '22 22:01 pavish

Blocked by https://github.com/centerofci/mathesar/pull/915

pavish avatar Jan 11 '22 22:01 pavish

I splintered off some of the content that was originally in the body of this ticket and put it into #1042

seancolsen avatar Feb 03 '22 18:02 seancolsen

This issue has not been updated in 90 days and is being marked as stale.

github-actions[bot] avatar Feb 04 '23 21:02 github-actions[bot]

This ticket is no longer required since our UX no longer contains such scenarios where InputGroup is used for the label and the component.

InputGroup now only groups inputs together, so focus is retained for each of the inputs independently.

pavish avatar Feb 06 '23 00:02 pavish