search

codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
28 stars 25 forks source link

Issue 286/new message styling #330

Closed andycwilliams closed 1 year ago

andycwilliams commented 1 year ago

Refactoring the new message function into a modal and applying MUI styling.

Opening this as a draft for now due to being away for a while and wanting to iron out any kinks. I'll have more time to sort through the most recent changes later, but wanted to update everyone what I'm working on right now.

2023-07-25 (6)

andycwilliams commented 1 year ago

Okay, still needs work (such as how clicking inside the modal opens and closes the messages in the background) and there are a couple of console errors to work through.

For me it's a bit of a toss-up between disabled and read-only, though I probably lean towards the latter. I think it should be immediately distinguishable from input that the user needs to provide, but still legible. disabled makes it a bit tougher to read than necessary, in my opinion. I'm trying a different variant for TextField for this as well.

Here it is with read-only: 2023-07-26 (4)

And here it is with disabled (with bonus errors for Grammarly that I've never seen before): 2023-07-26 (3)

xscottxbrownx commented 1 year ago

Okay, still needs work (such as how clicking inside the modal opens and closes the messages in the background) and there are a couple of console errors to work through.

For me it's a bit of a toss-up between disabled and read-only, though I probably lean towards the latter. I think it should be immediately distinguishable from input that the user needs to provide, but still legible. disabled makes it a bit tougher to read than necessary, in my opinion. I'm trying a different variant for TextField for this as well.

read-only looks good.

andycwilliams commented 1 year ago

When using the reply version of the modal, clicking around also opens and closes the messages in the background.

The issue is with the handleClick function on line 38 in MessagePreview.jsx.

Since that file is due for restyling anyway, we could leave this to be addressed at that point or have it be part of this PR as well.