Skip to content

feat: New option to make modal resizable #1885

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

Open
wants to merge 5 commits into
base: alpha
Choose a base branch
from

Conversation

damianstasik
Copy link
Contributor

@damianstasik damianstasik commented Oct 26, 2021

New Pull Request Checklist

Issue Description

This PR adds a "resize" handle to modals that have the resizable prop enabled.

Closes: #1879

Approach

TODOs before merging

  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 26, 2021

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@damianstasik damianstasik marked this pull request as ready for review October 27, 2021 18:22
@damianstasik
Copy link
Contributor Author

Here is how the final resize handle looks:
Screen Shot 2021-10-27 at 20 22 07

@damianstasik damianstasik requested a review from mtrezza October 27, 2021 18:23
@damianstasik
Copy link
Contributor Author

@mtrezza regarding labels being resized as well: I tried to force a max-width on them, but the Field component would require a slightly bigger refactor that I did not want to include in this PR.

@mtrezza
Copy link
Member

mtrezza commented Oct 27, 2021

That's fine then, thanks for looking into it.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

I tried it out and I wonder how we can combine the vertical and horizontal resizing by dragging the dialog corner. That the dialog can only be resized horizontally in the dialog corner and only vertically in the field corner is a rather unusual behavior.

There is also a bug that selects all text in the dialog when I resize horizontally:

Screen.Recording.2021-10-27.at.22.42.39.mov

@damianstasik
Copy link
Contributor Author

@mtrezza Maybe instead of the drag handle in the corner, we could allow resizing the modal with both the left and right edge of the container.

@mtrezza
Copy link
Member

mtrezza commented Oct 30, 2021

Not sure how the experience of that would be. I can only think of a corner drag handle, like the field had. That is the UI behavior a user would typically expect. If there is another way that is intuitive, we can also explore that.

@damianstasik
Copy link
Contributor Author

It could work as in a regular OS window manager, the whole side could be used to resize the modal:

sideresizable.mov

@mtrezza
Copy link
Member

mtrezza commented Oct 30, 2021

If it's separately resizable on the horizontal and vertical side it would certainly be an improvement over the status quo, but it would still be impractical when you cannot resize both simultaneously. I think that's an issue when want to resize because there is a large JSON object that you want to display and you want to fit the dialog just right to display it.

What is the difficulty to let resize both at the same time? Is it a component restriction?

Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: new option to make modal resizable feat: New option to make modal resizable Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow config parameter dialog to be resized in width
3 participants