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

feat(shell-panel): Add resizable property to allow resizing of content. #2770 #3535

Merged
merged 17 commits into from
Nov 29, 2021

Conversation

driskull
Copy link
Member

@driskull driskull commented Nov 18, 2021

Related Issue: #2770

Summary

feat(shell-panel): Add resizable property to allow resizing of content.

Todo

Styling

@github-actions github-actions bot added this to the Sprint 11/8 - 11/19 milestone Nov 18, 2021
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Nov 18, 2021
@driskull driskull requested a review from asangma November 18, 2021 20:04
@driskull driskull added the 2 - in development Issues that are actively being worked on. label Nov 18, 2021
@driskull driskull marked this pull request as ready for review November 18, 2021 22:23
@driskull driskull requested a review from a team as a code owner November 18, 2021 22:23
@driskull
Copy link
Member Author

@asangma can you review the DOM structure and advise on styling?

@jcfranco can you review zeee codez?

@driskull driskull requested a review from jcfranco November 19, 2021 22:13
@macandcheese
Copy link
Contributor

This is awesome - really like the attention paid to home / end / shift + nudges. Want me to take a crack at styling?

@driskull
Copy link
Member Author

This is awesome - really like the attention paid to home / end / shift + nudges. Want me to take a crack at styling?

Yes plz!

@macandcheese
Copy link
Contributor

Do you think adding an event that emits the px width at interaction end would be useful? If an app wants to save a user's last width, for example.

@driskull
Copy link
Member Author

Yeah, i was considering an event. I think we could add it if we think its beneficial. However, nowadays users can just use a resize observer too.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome! Just had some suggestions and nitpicks. Looks like a great enhancement! I'll defer to @Esri/calcite-design-representatives for the styling-side of things.

@driskull
Copy link
Member Author

@jcfranco checked in fixes.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🎉 🌮 🦖

@macandcheese
Copy link
Contributor

@asangma did you have any thoughts on design? I think it's tough to use any kind of handle affordance because of potential overlap with content.

Below shows the separator while hovering anywhere in the panel, to increase visibility of feature a bit. I looked at many examples of this functionality and most do not provide anything except the hover state while over the 1px separator. Below is 2px, feel free to provide feedback here!

https://jmp.sh/6gcuI7X

@driskull
Copy link
Member Author

I think it's tough to use any kind of handle affordance because of potential overlap with content.

Maybe we should just have an invisible div that is positioned above the content on the side?

@macandcheese
Copy link
Contributor

macandcheese commented Nov 23, 2021

Maybe we should just have an invisible div that is positioned above the content on the side?

We could do that but I kind of like just using the .separator div. I've struggled to find an example of a resizable panel that does have some "extra handle" or anything like that, even in native OS it is usually a cursor change and border color. Open to using different color values for that cc @asangma

@asangma
Copy link
Contributor

asangma commented Nov 23, 2021

Hmmm...my initial thought is to have the "grab-able" area fatten up on focus/hover. But I'll pull and build and have a look.

@asangma
Copy link
Contributor

asangma commented Nov 23, 2021

As far as the .separator node, it has some issues when content-behind="false".

@driskull
Copy link
Member Author

@asangma content-behind="false" shouldn't be a thing. Do you mean content-behind?

@macandcheese
Copy link
Contributor

Let me know if I should create a branch with the above video styles from this one. I think the 2px / hover and focus affordance while over shell panel is a good middle ground, I worry about jumpiness if that pixel width increases further on hover.

@driskull
Copy link
Member Author

@macandcheese just check them in here. I don't think we need a separate branch.

@macandcheese
Copy link
Contributor

Added styles if ya want to take another look. The content-behind issue @asangma mentioned is when a panel is detached - the separator is still full-height currently. It's kind of an edgy case - and is true to the actual space occupied by the shell panel container - maybe not the worst thing but open to thoughts there:
Screen Shot 2021-11-29 at 10 42 43 AM

@driskull
Copy link
Member Author

Do we just want to disable resizing when a panel is detached?

@asangma
Copy link
Contributor

asangma commented Nov 29, 2021

Do we just want to disable resizing when a panel is detached?

yeah...I think so.

@asangma
Copy link
Contributor

asangma commented Nov 29, 2021

content-behind="false" shouldn't be a thing. Do you mean content-behind?

I meant when content-behind is not set.

Copy link
Contributor

@asangma asangma left a comment

Choose a reason for hiding this comment

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

image
YODA MAN.

@driskull driskull merged commit c08d4c4 into master Nov 29, 2021
@driskull driskull deleted the dris0000/shell-panel-resizable branch November 29, 2021 22:34
driskull added a commit that referenced this pull request Dec 12, 2024
**Related Issue:** #7591

## Summary

- adds resize functionality
- adds assistive text live region for keyboard instructions
- adds tests
- updates story
- adds dom utility
- adds t9n files
- use new resize icon
- uses separator role, related to #3535
benelan pushed a commit that referenced this pull request Feb 8, 2025
**Related Issue:** #7591

## Summary

- adds resize functionality
- adds assistive text live region for keyboard instructions
- adds tests
- updates story
- adds dom utility
- adds t9n files
- use new resize icon
- uses separator role, related to #3535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - in development Issues that are actively being worked on. enhancement Issues tied to a new feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants