human-essentials icon indicating copy to clipboard operation
human-essentials copied to clipboard

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

Open AweysAhmed opened this issue 1 year ago • 6 comments

Resolves #4242

Description

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

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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

  • set up a new distribution (Distributions | New Distribution) , a new donation (Donation |New Donation), and a new purchase (Purchases | New Purchase) that are all using a particular item (e.g. Adult Briefs L/XL)

  • then go into Inventory | Inventory Adjustments

  • Enter new Inventory Adjustments to take the levels for that item for each storage location down to 0

  • Go to Inventory|Items & Inventory

  • Deactivate the item

  • Go to Distributions

  • you should see a "Has Inactive Items" message. The style of this has has be changed

  • click View on that item, and scroll to the bottom

  • you should see a message indicating why you can't correct this. The style of this message has be changed

  • Go to Donations

  • Click view on the donation you entered and scroll to the bottom

  • you should see a message indicating why you can't correct this. The style of this message has be changed

  • Go to Purchases

  • Click view on the purchase you entered and scroll to the bottom

  • you should see a message indicating why you can't correct this. The style of this message has be changed

  • Before

    Screenshot 2024-06-19 at 1 12 38 PM Screenshot 2024-06-23 at 4 11 42 PM Screenshot 2024-06-23 at 4 15 24 PM

    After

    Screenshot 2024-06-19 at 10 01 38 PM

    Screenshot 2024-06-23 at 4 13 55 PM Screenshot 2024-06-23 at 4 15 37 PM

AweysAhmed avatar Jun 20 '24 02:06 AweysAhmed

@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'.

cielf avatar Jun 23 '24 23:06 cielf

@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.

AweysAhmed avatar Jun 24 '24 17:06 AweysAhmed

@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.

cielf avatar Jun 24 '24 17:06 cielf

@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 avatar Jun 24 '24 17:06 AweysAhmed

@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.

AweysAhmed avatar Jun 24 '24 18:06 AweysAhmed

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.

cielf avatar Jun 24 '24 21:06 cielf

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.

@cielf @dorner I have added a missing screenshot and double-checked to make sure the CSS class .warning isn't used anywhere else.

Since we are only using it the erb files that I have changed I have renamed the CSS class to .low_priority_warning and remove the red colour so that if we need a more urgent warning CSS class we can create a .warning class.

AweysAhmed avatar Jul 02 '24 16:07 AweysAhmed

@AweysAhmed: Your PR Fixes #4242 so that css file doesn't make the warning message look scary is part of today's Human Essentials production release: 2024.07.07. Thank you very much for your contribution!

github-actions[bot] avatar Jul 07 '24 14:07 github-actions[bot]