search

gridhead / syngrafias

A free and open-source synchronized authorship web-application for seamlessly collaborating on documents using intuitive cells
GNU General Public License v3.0
15 stars 12 forks source link

Variable cell partition width #71

Closed VaibhavSaini19 closed 3 years ago

VaibhavSaini19 commented 3 years ago

Description

This PR fixes #59

Have you read the Contributing Guidelines before opening this PR

Yes

Notes for Reviewers

Since Split.js modifies the width of the cell partition, I had to remove the ui grid , eight column wide and sixteen column wide classes and use display: flex.

P.S. Isn't the textarea supposed to occupy the full height? It has been given 2 rows height by default

Screenshot for changes made

 

Screenshot (187)

gridhead commented 3 years ago

Hi @shivangswain @VaibhavSaini19 @s-katte @khallnayak, please go ahead and review this PR.

Since Split.js modifies the width of the cell partition, I had to remove the ui grid , eight column wide and sixteen column wide classes and use display: flex.

Which basically undoes the consequences of #58, right?

P.S. Isn't the textarea supposed to occupy the full height? It has been given 2 rows height by default

Yes it is supposed to. Please make the necessary changes to accompany those as well.

VaibhavSaini19 commented 3 years ago

Which basically undoes the consequences of #58, right?

I already made the necessary changes to maintain the desired functionality. It switches between 50-50 width, 100% width to textarea cell and 100% width to output cell. It is just that it won't remember your manual width partition if you cycle toggle

Yes it is supposed to. Please make the necessary changes to accompany those as well.

Should I make a new issue regarding that, since it is a problem on its own, unrelated to this PR?

gridhead commented 3 years ago

It is just that it won't remember your manual width partition if you cycle toggle

Not a problem. It is something we can live with.

Should I make a new issue regarding that, since it is a problem on its own, unrelated to this PR?

Please go ahead and create an issue regarding the same. Remember that the issue would garner prevalence only when #71 is merged.

gridhead commented 3 years ago

Hi @shivangswain @s-katte @tripathyprateek, I could use your reviews here. Thank you.

VaibhavSaini19 commented 3 years ago

@t0xic0der its working fine on my end. @s-katte can you confirm if it is working as expected?

shivangswain commented 3 years ago

Works great for me too.