compose: Add support for full-screen compose box #17667
Conversation
Signior-X
force-pushed
the
compose-size
branch
2 times, most recently
from
f587d56
to
e0ed88f
Compare
Signior-X
changed the title
Signior-X
force-pushed
the
compose-size
branch
4 times, most recently
from
e90ec35
to
00a1562
Compare
Signior-X
force-pushed
the
compose-size
branch
2 times, most recently
from
f31fb92
to
0c558c3
Compare
|
@timabbott made the buttons with different IDs and commented above. Also rebased this, ready for another review now. |
static/styles/compose.css
Outdated
| #compose-content { | ||
| height: 100%; | ||
| display: flex; | ||
| flex-flow: column; |
There was a problem hiding this comment.
What is the flex logic needed for?
I would hope that we can figure out how to reduce the amount of special CSS for this mode, e.g. if there are refactors we can do to the existing compose box CSS that would make these unnecessary.
There was a problem hiding this comment.
flex logic is needed for automatically setting the maximum height possible in the compose box. Like I give a height to #compose and then, make flex inside it. The element that needs to be stretched are given flex: 1 to automtically fill all the spaces left. and height: 100% so, that height comes down.
This makes it possible to make the height fit according to percentage of height available and parent elements.
I changed the approach for the following reasons:
- The alerts get handled automatically.
- The elements position itself, so no need to calculate height.
- This way also makes the height change automatically even without autosize in full screen mode if the device height is changed.
The approach of css can be simplified as start with #compose with the height the element that is fixed. Now for every element inside it, check if it is unique or have multiple children. Then look for which children needs to get stretched to obtain the left over space and give flex: 1 to it. Following this sequence, the textarea finally got the height to get maximum stretch possible and it will handle it properly.
There was a problem hiding this comment.
I will see for the refracter tomorrow.
There was a problem hiding this comment.
This is possible, clearly. Refractored accordingly.
There was a problem hiding this comment.
I have added another commit, so that you can see if that looks good.
|
This looks pretty promising @Signior-X! I think we should be about ready to merge this, or at least test-deploy on chat.zulip.org once the above comments are resolved. |
|
@timabbott squashed the commits and commented above for the queries. |
Signior-X
force-pushed
the
compose-size
branch
2 times, most recently
from
6be2469
to
da9bf7d
Compare
|
@timabbott updated the PR accordingly, made changes accordingly. The conversations that still unresolved are above, need feedback/review on them. |
Signior-X
force-pushed
the
compose-size
branch
2 times, most recently
from
377e0b0
to
43af1db
Compare
|
@timabbott rebased this and now this is ready for review. Commented above for all the queries. |
| autosize($("#compose-textarea")); | ||
|
|
||
| $(".collapse_composebox_button").hide(); | ||
| $(".expand_composebox_button").show(); |
There was a problem hiding this comment.
I added a line in both of these functions to move focus back to the compose box, since that's almost always what you want.
This commit makes a working toggler in compose_actions that adds the compose-fullscreen class to the compose that removes the max-height from the compose textarea and adds flex elements above so that the height automatically adjust with the device height. This results in making the compose box full screen sized. The compose_height.js maintains the state of the height of the compose box. Also, when the compose box is closed, the compose box is reset to it's default behaviour and original height. So, everytime user need not toggle off the compose full size and only for specific message it is used. It also adds destroy autosize on compose_height state change. It destroys the autosize of textarea when the full screen sized compose box is toggled on. And everytime when it is turned off, it reinitialises the autosize. This also adds a condition in autosize_textarea to only autosize when composebox is not in full height state. Fixes zulip#17660
This is by no means the ideal solution, since we are likely to occlude your cursor, but it prevents the user experience from being clearly broken.
|
OK, I've merged this with the above changes. I think while we're thinking about this, the |
Fixes #17660
Testing plan:
Tested manually:
Open the compose box, toggle the height using the toggler button added just below the compose area.
To check autosize doesnot interfare is that when the compose height state is true, it is destroyed as this is the way to stop all the listeners added by it (check on docs of autosize) and particularly checked for changing messages and drafts while having full size compose box.
The save and copy also doesn't not have any problems as the autosize has been destroyed.
For node tests, I think for the compose_height file some basic tests needs to be defined.
GIFs or screenshots:
The design of full height text-area currently.