codeforpdx / PASS

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

Issue 399/create message read status #400

Closed leekahung closed 1 year ago

leekahung commented 1 year ago

Issue 399/create message read status

This PR:

1. Includes a new read status as part of message object 2. Utilizes the new read status to indicate messages that has been read by altering the opacity of the header section of the message preview 3. Made changes compatible with older message objects that doesn't have read status by making them false by default then including a new read status as part of the message 4. Made changes compatible with older message objects that doesn't have read status by generating a new read status property for the message object when message preview is clicked and is set to true


The files this PR effects:

Components

Other Files


Screenshots (if applicable):

When read status is updated:

https://github.com/codeforpdx/PASS/assets/14917816/aa109f0f-4bed-46ad-9094-aa0543a0cab6

When read status is updated on older message objects from Solid:

https://github.com/codeforpdx/PASS/assets/14917816/04125daf-6d49-4b60-9427-f275adbc4896


Future Steps/PRs Needed to Finish This Work (optional):


Issues needing discussion/feedback (optional):

1. PR related to Issue #399

xscottxbrownx commented 1 year ago

I really like that you went with just changing opacity over making a NEW icon or word or something!


1) Do you have any thoughts as to why the first video seems to change opacity immediately, and second video takes a while?

2) May be just out of scope for this PR, but thoughts on using a .map() in MessagePreview on the Grid items - so if we make future changes it doesn't need to be written 3 times? Something like:


const messageInfo = [
    {
        title: 'Sender: ',
        text: {message.sender}
    },
    {
        title: 'Subject: ',
        text: {message.title}
    },
    {
        title: 'Date: ',
        text: {message.uploadDate.toLocaleDateString()}
    }
];

{messageInfo.map( message => (
    <Grid item xs={3} sx={{ opacity: message.readStatus ? '0.5' : '1' }}>
        <Typography> {message.title} <strong> {message.text} </strong> </Typography>
     </Grid>   
))

It's just a thought. We'd be adding more lines now (due to setting up the array) - but MAY be easier to maintain in future.

leekahung commented 1 year ago
  1. Do you have any thoughts as to why the first video seems to change opacity immediately, and second video takes a while?

Yeah, I think it's because the first one was part of the new updates where the read status property already exist, so it's just a matter of switching the value from false to true.

However in the older message object in the second clip, the read status doesn't exist at all. So the function has to generate a new Thing for the RDF, which then gets reflected as read status true.

  1. May be just out of scope for this PR, but thoughts on using a .map() in MessagePreview on the Grid items - so if we make future changes it doesn't need to be written 3 times?

It's just a thought. We'd be adding more lines now (due to setting up the array) - but MAY be easier to maintain in future.

No problem with using .map, think it's a good idea. I'll refactor it here with this PR.

xscottxbrownx commented 1 year ago

Yeah, I think it's because the first one was part of the new updates where the read status property already exist, so it's just a matter of switching the value from false to true.

However in the older message object in the second clip, the read status doesn't exist at all. So the function has to generate a new Thing for the RDF, which then gets reflected as read status true.

ahh yes. Makes sense. Thank you.