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 SimpleFormIterator UI #8124

Merged
merged 12 commits into from
Sep 2, 2022
Merged

Improve SimpleFormIterator UI #8124

merged 12 commits into from
Sep 2, 2022

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Aug 31, 2022

  • Remove line index by default
  • Use the same type of button for all actions
  • Show row actions only on hover (except on Mobile)
  • Add a fullWidth prop to push actions to the row end
  • Emphasize the inline layout in the documentation
  • Fix label position
  • Fix layout for nested ArrayInput
  • Fix tests
  • Add more stories

Before

Capture d’écran 2022-08-31 à 17 24 31

After

ezgif com-gif-maker (5)

The docs introduce this component with a nicer screenshot:

array-input

@fzaninotto
Copy link
Member Author

The rationale for separating the remove button from the others in the first place was that it's destructive. Maybe we should keep that. Let me think again about the best compromise.

Back to WIP.

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Very nice so far!! 😍

@slax57
Copy link
Contributor

slax57 commented Aug 31, 2022

The rationale for separating the remove button from the others in the first place was that it's destructive. Maybe we should keep that. Let me think again about the best compromise.

Back to WIP.

The different colors still make it clear enough, imho

@WiXSL
Copy link
Contributor

WiXSL commented Aug 31, 2022

Woow this is great!

@fzaninotto fzaninotto added RFR Ready For Review and removed WIP Work In Progress labels Sep 1, 2022
@fzaninotto
Copy link
Member Author

OK, I've made up my mind, this is back to RFR!

docs/SimpleFormIterator.md Outdated Show resolved Hide resolved
docs/SimpleFormIterator.md Outdated Show resolved Hide resolved
Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

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

Looking at the ArrayInput stories, specially Nested Inline, the inputs look to dense.
And the separation line between items gets too close to the inputs

fzaninotto and others added 2 commits September 1, 2022 18:06
Co-authored-by: Aníbal Svarcas <WiXSL@users.noreply.github.com>
@fzaninotto
Copy link
Member Author

Looking at the ArrayInput stories, specially Nested Inline, the inputs look to dense.
And the separation line between items gets too close to the inputs

This story shows the "densest" density we can get. I still think it's not dense enough ;)

You can see the default density in the Basic story, and I don't think there is a density problem there. On the contrary, I find it barely usable because everything is so far from everything.

image

As for the separator line, I'm not 100% satisfied with the result either, but the alternatives I tested were worse.

@slax57 slax57 added this to the 4.4.0 milestone Sep 2, 2022
@slax57 slax57 merged commit 703db10 into next Sep 2, 2022
@slax57 slax57 deleted the simpleformiterator-ui branch September 2, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants