style(ui-markdown-editor): resolve toolbar icon shrinkage - I228

[K-Kumar-01]

Closes #228

The toolbar icons in markdown shrank too much. Resolved that issue.

Changes

• Changed the stylings of toolbar icons
• Used media queries to achieve this.

Flags

• The separator has a height:50px that may seem a bit large in some small area range.
• Devices less than 330px will have an overflow of the toolbar.

Screenshots or Video

Author Checklist

• Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
• Vital features and changes captured in unit and/or integration tests
• Commits messages follow AP format
• Extend the documentation, if necessary
• Merging to master from fork:branchname
• Manual accessibility test performed
  • Keyboard-only access, including forms
  • Contrast at least WCAG Level A
  • Appropriate labels, alt text, and instructions

[jolanglinais]

We should have addressed this in #228, but any thoughts on how this should ideally be @Michael-Grover? My first thought is having a minimum width for the whole editor, so the icons can not need to resize or anything.

[K-Kumar-01]

We should have addressed this in #228, but any thoughts on how this should ideally be @Michael-Grover? My first thought is having a minimum width for the whole editor, so the icons can not need to resize or anything.

@irmerk
The icons will shrink when the window is resized so I don't think this will be a good approach.
I don't have any other idea about how this should be tackled though

[Michael-Grover]

@irmerk I agree that a minimum width for the editor would make sense. @K-Kumar-01 How about making the minimum the same width as the contract editor component? It looks like that's 670px.

[K-Kumar-01]

@irmerk I agree that a minimum width for the editor would make sense. @K-Kumar-01 How about making the minimum the same width as the contract editor component? It looks like that's 670px.

@Michael-Grover
I cannot understand what to do. Can you explain please as I am confused about what to do? 😅

[Michael-Grover]

@K-Kumar-01 I'm not sure what the best way to do this is from a coding perspective, but what I'm suggesting is that we:

Make this sure this component can't reduce to a width smaller than 670px. When the screen is too narrow to accommodate the 670px width component, the component will overflow off the right side of the screen. For example, if the viewport is 400px width, the component will be 670px width, however if the viewport is 1200px width, the component will grow in size to fit the screen like it currently does. This will ensure the toolbar is never so narrow that the icons shrink.

[K-Kumar-01]

@K-Kumar-01 I'm not sure what the best way to do this is from a coding perspective, but what I'm suggesting is that we:

Make this sure this component can't reduce to a width smaller than 670px. When the screen is too narrow to accommodate the 670px width component, the component will overflow off the right side of the screen. For example, if the viewport is 400px width, the component will be 670px width, however if the viewport is 1200px width, the component will grow in size to fit the screen like it currently does. This will ensure the toolbar is never so narrow that the icons shrink.

@Michael-Grover
Well, thanks for the explanation. I had a few doubts and queries. On mobile screens wouldn't it be uncomfortable? Besides, I think that it may cause some styling problems though I am not sure about it.
I would like to draw attention to one more thing that 670px +210px i.e. a total of 880px or greater would be required for the working when the sidebar is shown.
So I am not fully confident about this approach.
Would like suggestions from @irmerk also on how can we go.

[jolanglinais]

I don't think we should be concerned with mobile usage. This repository is not geared towards that end. In the future, we can approach mobile functionality.

In the meantime, I think we should set a minimum width for the editor and toolbar and let if overflow if the page width is reduced to less than that. This should remove the issue of icons resizing because nothing can go lower in width than what we specify.

[K-Kumar-01]

I don't think we should be concerned with mobile usage. This repository is not geared towards that end. In the future, we can approach mobile functionality.

In the meantime, I think we should set a minimum width for the editor and toolbar and let if overflow if the page width is reduced to less than that. This should remove the issue of icons resizing because nothing can go lower in width than what we specify.

@irmerk @Michael-Grover
I have tried implementing the above.

Attaching screenshots for reference.

If it is suitable, I will commit it otherwise make any more changes needed.
I used js for styling as the styles applied were to the body tag and a slate component which I could not figure out to apply.
The styles though will be applied only here using useEffect.

Code

useEffect(()=>{
    const toolbar = document.getElementById("ap-rich-text-editor-toolbar");
    toolbar.parentElement.style.minWidth = "630px";
    document.body.style.overflowX = "auto";
  },[])

Let me know if there are any changes required.

[Michael-Grover]

Hi @K-Kumar-01 , I opened the Netlify link and I'm somehow still able to reduce the width of the component below the minimum we set. @irmerk are you seeing the same behavior?

[jolanglinais]

@K-Kumar-01 could you add your commits to the PR so we can view the changes?

[K-Kumar-01]

@irmerk @Michael-Grover
I have added my latest commits to the PR.
Let me know if there are any changes required.
I apologize for the earlier incident @Michael-Grover. I forgot to push the commits 😅

[Michael-Grover]

looks good to me, thanks 👍 . @irmerk may want to review as well.

[jolanglinais]

I don't think this is the right approach. I. think the consumer of the ui-markdown-editor component should determine the overflow. In this instance that would be the Storybook. Also, you've already set the min-width on the Menu, so this useEffect isn't necessary.

[K-Kumar-01]

@irmerk

I. think the consumer of the ui-markdown-editor component should determine the overflow. In this instance that would be the Storybook.

I am not quite getting what you are trying to say. 😅
Could you please tell me about this?

As for the useEffect part, the minWidth only makes the toolbar to a min-width.
The area that will contain the text (where the slate tag is used) won't be shrinked. So I had to resort to useEffect for that part

[K-Kumar-01]

@irmerk

I. think the consumer of the ui-markdown-editor component should determine the overflow. In this instance that would be the Storybook.

I am not quite getting what you are trying to say.
Could you please tell me about this?

As for the useEffect part, the minWidth only makes the toolbar to a min-width.
The area that will contain the text (where the slate tag is used) won't be shrinked. So I had to resort to useEffect for that part

@irmerk
Any updates on this one?

[jolanglinais]

Yes the ui-markdown-editor is a component, which is used in the Storybook and ui-contract-editor, or any other app that decides to use it. Those components are the ones that should decide the overflow of ui-markdown-editor when they render it.

[K-Kumar-01]

Yes the ui-markdown-editor is a component, which is used in the Storybook and ui-contract-editor, or any other app that decides to use it. Those components are the ones that should decide the overflow of ui-markdown-editor when they render it.

@irmerk
So basically the component which is currently active i.e. ui-markdown-editor or ui-contract-editor should be responsible for the
resize?
Am I correct here?

[jolanglinais]

Yes I believe that is correct.

[K-Kumar-01]

Yes I believe that is correct.

@irmerk @Michael-Grover
I have changed the approach as suggested by @irmerk .
There are global body styles added for the storybook to see overflow and div added only for markdown editor.
Let me know if there are any changes required :)

[Michael-Grover]

This looks good to me, thanks!

[jolanglinais]

This still doesn't look like the right approach to me - wrapping the editor in a div with a min-width. I'll look into this more when I have a chance.

[jolanglinais]

Sorry I haven't had much time to look into this more before and haven't been able to give very helpful guidance. I didn't look at this well enough and that resulted in this idea.

But I believe all we really need is to configure min-width: 600px in these two places. We don't need overflow-x: auto or anything else.

[K-Kumar-01]

Sorry I haven't had much time to look into this more before and haven't been able to give very helpful guidance. I didn't look at this well enough and that resulted in this idea.

But I believe all we really need is to configure min-width: 600px in these two places. We don't need overflow-x: auto or anything else.

@irmerk
If overflow-x:auto is not set then on resizing window below a certain size the content gets cut off in my implementations. I will try to implement the suggested changes but I think that overflow-x:auto would be required. Will confirm and tell you after the implementation.

[jolanglinais]

Ah, well that should be added in the storybook, not on ui-markdown-editor

[K-Kumar-01]

@irmerk and @Michael-Grover
I have made some changes to the PR.
Basically providing min-width to toolbar menu and text editor separately.
Let me know if there are any changes required.

[jolanglinais]

Great, thanks for your patience! Want to get @DianaLease's 👀 on this as well.