Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve focus indicator of InputGroup component #971

Closed
pavish opened this issue Jan 11, 2022 · 4 comments
Closed

Improve focus indicator of InputGroup component #971

pavish opened this issue Jan 11, 2022 · 4 comments
Labels
ready Ready for implementation type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@pavish
Copy link
Member

pavish commented Jan 11, 2022

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 pavish added type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory needs: unblocking Blocked by other work labels Jan 11, 2022
@pavish pavish added this to the [07.2] 2022-02 improvements milestone Jan 11, 2022
@pavish
Copy link
Member Author

pavish commented Jan 11, 2022

Blocked by #915

@pavish pavish mentioned this issue Jan 11, 2022
7 tasks
@seancolsen seancolsen added ready Ready for implementation and removed needs: unblocking Blocked by other work labels Feb 2, 2022
@seancolsen seancolsen mentioned this issue Feb 2, 2022
7 tasks
@seancolsen seancolsen changed the title InputGroup component improvements Improve focus indicator of InputGroup component Feb 3, 2022
@seancolsen
Copy link
Contributor

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

@kgodey kgodey modified the milestones: [06.2] 2022-04 improvements, [08.1] 2022-05 improvements May 2, 2022
@seancolsen seancolsen modified the milestones: [08.1] 2022-05 improvements, [v2] Future Consideration May 13, 2022
@kgodey kgodey modified the milestones: [v2] Future Consideration, Unprioritized Jun 1, 2022
@github-actions
Copy link

github-actions bot commented Feb 4, 2023

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

@github-actions github-actions bot added the stale label Feb 4, 2023
@pavish
Copy link
Member Author

pavish commented Feb 6, 2023

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 pavish closed this as completed Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Ready for implementation type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
Development

No branches or pull requests

3 participants