search

accordproject / markdown-editor

Markdown editor based on Slate.js
Apache License 2.0
42 stars 47 forks source link

fix(styles): fix markdown editor width - I334 #338

Closed sachdeva-shrey closed 4 years ago

sachdeva-shrey commented 4 years ago

Signed-off-by: Shrey Sachdeva shreysachdeva.2000@gmail.com

Issue #334

Fix Markdown Editor width so it doesn't overlap with Rich Text Editor on resizing.

Changes

Flags

jolanglinais commented 4 years ago

~Note to reviewers: Will need to test within cicero-ui as well. Related to #315 so review in conjunction.~

Nevermind, this is the MarkdownEditor not the RichTextEditor

jolanglinais commented 4 years ago

@shrey-sachdeva2000 can this just be forced in within the demo?

DianaLease commented 4 years ago

@shrey-sachdeva2000 can this just be forced in within the demo?

I agree with this ^ The component itself should not limit its width because others using it may not want that limitation. Let's just style it within the context of the demo.

sachdeva-shrey commented 4 years ago

Sure! That makes sense!

sachdeva-shrey commented 4 years ago

If we set max-width: 100% instead of max-width: 45vw the textarea would stay within its parent element. Would that be a good option?

DianaLease commented 4 years ago

If we set max-width: 100% instead of max-width: 45vw the textarea would stay within its parent element. Would that be a good option?

Yes, that sounds good to me!

DianaLease commented 4 years ago

If we set max-width: 100% instead of max-width: 45vw the textarea would stay within its parent element. Would that be a good option?

Sorry, looking at this code again. Setting max-width is kind of redundant since the width is already set to 100%. What is the issue exactly? When I resize the page in the currently deployed demo, the two components never overlap in the demo.

I see now, the resizable tab is what does it. Should we just remove the resizable tab? I don't think it adds any value. @irmerk ? @Michael-Grover ?

@shrey-sachdeva2000 FYI I edited my comment above.

sachdeva-shrey commented 4 years ago

Sorry, looking at this code again. Setting max-width is kind of redundant since the width is already set to 100%. What is the issue exactly? When I resize the page in the currently deployed demo, the two components never overlap in the demo.

It is actually after one tries to resize the MarkdownEditor textarea that it flows out of its set width and it's not resizable again. Not sure if this happens to be an issue with a specific browser itself. Please check the screenshot here: https://github.com/accordproject/markdown-editor/issues/334

jolanglinais commented 4 years ago

Yeah agree with @DianaLease, make this all static and get rid of resizing and such. It's a demo and should just be static.

jolanglinais commented 4 years ago

Looks good to me, just rebase to be up to date with master

sachdeva-shrey commented 4 years ago

Looks good to me, just rebase to be up to date with master

Sure, the PR is now ready to merge.

jolanglinais commented 4 years ago

You'll want to get rid of fc5c50b, we don't want merge commits in the git log. I think always lean towards rebasing.

sachdeva-shrey commented 4 years ago

Ohh my bad. It's fixed now!