rubyforgood / human-essentials

Human Essentials is an inventory management system for diaper, incontinence, and period-supply banks. It supports them in distributing to partners, tracking inventory, and reporting stats and analytics.
https://humanessentials.app
MIT License
436 stars 450 forks source link

Fixes #4242 so that css file doesn't make the warning message look scary #4462

Open AweysAhmed opened 1 week ago

AweysAhmed commented 1 week ago

Resolves #4242

Description

This PR updates the CSS so that they warning text appears in plaintext and not in red.

Type of change

How Has This Been Tested?

I tested this changes locally to ensure removing the text-colour changes the warning message to black or close to black.

In order to test this out you will need to

cielf commented 5 days ago

@AweysAhmed I'll take a better look at this tomorrow. My initial thoughts are 'but I think there are other warnings, that might need more than plain text', and 'maybe we need a different name for this to indicate that it's a low-intensity warning'.

AweysAhmed commented 4 days ago

@AweysAhmed I'll take a better look at this tomorrow. My initial thoughts are 'but I think there are other warnings, that might need more than plain text', and 'maybe we need a different name for this to indicate that it's a low-intensity warning'.

That is a good point. I was thinking of changing the class CSS class name to div.low_priority_warning and that way we can create another class for medium and hight priority warnings.

cielf commented 4 days ago

@AweysAhmed I'll take a better look at this tomorrow. My initial thoughts are 'but I think there are other warnings, that might need more than plain text', and 'maybe we need a different name for this to indicate that it's a low-intensity warning'.

That is a good point. I was thinking of changing the class CSS class name to div.low_priority_warning and that way we can create another class for medium and hight priority warnings.

May i suggest that, instead, you create a new class for div.low_priority_warning, and leave div.warning as it was. That way, if there is anything else that is using the warning class, it won't be affected.

AweysAhmed commented 4 days ago

@AweysAhmed I'll take a better look at this tomorrow. My initial thoughts are 'but I think there are other warnings, that might need more than plain text', and 'maybe we need a different name for this to indicate that it's a low-intensity warning'.

That is a good point. I was thinking of changing the class CSS class name to div.low_priority_warning and that way we can create another class for medium and hight priority warnings.

May i suggest that, instead, you create a new class for low_priority_warning, and leave warning as it was. That way, if there is anything else that is using the warning class, it won't be affected.

Thank you. That makes sense. I'll make that change.

AweysAhmed commented 4 days ago

@AweysAhmed I'll take a better look at this tomorrow. My initial thoughts are 'but I think there are other warnings, that might need more than plain text', and 'maybe we need a different name for this to indicate that it's a low-intensity warning'.

That is a good point. I was thinking of changing the class CSS class name to div.low_priority_warning and that way we can create another class for medium and hight priority warnings.

May i suggest that, instead, you create a new class for low_priority_warning, and leave warning as it was. That way, if there is anything else that is using the warning class, it won't be affected.

Thank you. That makes sense. I'll make that change.

I forgot to mention that the current div.warning CSS class is only used in the three locations that display the warning messages that need to be updated. So my changes will not impact any other erb files.

So maybe we should create the div.low_priority_warning and if in the future we need a more urgent warning message we can create it.

cielf commented 4 days ago

Sure -- if we're sure it's not used elsewhere. I know there are other warning-type messages, but they may be using a different class.