Safari CSS gradient bug #14311

[valtism]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update

Description

Transition to a transparent version of the same background color with
no opacity so that safari transitions through correctly.

Switch all --story-comments-bg themes to hexadecimal, fixes no fade bug
in "Pink" theme.

Related Tickets & Documents

Closes #14311

QA Instructions, Screenshots, Recordings

Please see the linked issue for screenshots of the issue and how it should look with this fix.

Added/updated tests?

• Yes
• No, and this is why: I don't think this CSS change needs tests.
• I need help with writing tests

[github-actions]

Thank you for opening this PR! We appreciate you!

For all pull requests coming from third-party forks we will need to
review the PR before we can process it through our CI pipelines.

A Forem Team member will review this contribution and get back to
you as soon as possible!

[valtism]

Apologies, was updating the last PR and didn't realise that if I force-pushed then git would no longer be able to track it in the same PR.

@ludwiczakpawel I addressed the points you made here: #14893 (review)

I would have liked to use the premade colours with opacity, but the base values I was working with were already flat variants based off base colours with some transparency. They could not be used like that because they were being used for a fade overlay and had to transition from totally opaque to transparent.

[aitchiss]

This is looking good, but I noticed it looks like the background of the overall comments container has changed, so now the gradient looks a bit like it's the wrong colour (since the colour only becomes visible right at the end) 🤔

Light before:

Light after:

Dark before:

Dark after:

[valtism]

@aitchiss Thanks so much for testing this 🙏 I tried to set the repo up locally, but ran into lots of errors. Since it was a simple change, I thought I could pull it off without making mistakes but here we are. Just needed to change once var ref to process the new color with rgb() since we're not using hex values any more.

[aitchiss]

Looks like some of the tests are failing with e.g.

An error occurred while loading ./spec/services/users/create_mascot_account_spec.rb.
Failure/Error: background: rgb(var(--story-comments-bg));
SassC::SyntaxError:
  Error: Function rgb is missing argument $green.
          on line 30:9 of app/assets/stylesheets/crayons.scss
          from line 2:9 of app/assets/stylesheets/admin.scss
  >>     background: rgb(var(--story-comments-bg));
     ----------------^
# ./app/assets/stylesheets/components/stories.scss:196
# ./app/lib/url.rb:71:in `local_image'
# ./app/models/settings/general.rb:49:in `block in <class:General>'
# ./app/models/settings/base.rb:70:in `block in define_setting'
# ./app/services/users/create_mascot_account.rb:7:in `<class:CreateMascotAccount>'
# ./app/services/users/create_mascot_account.rb:2:in `<module:Users>'
# ./app/services/users/create_mascot_account.rb:1:in `<main>'
# ./spec/services/users/create_mascot_account_spec.rb:3:in `<main>'

I had a quick check locally and it seems like in sass when you pass a CSS variable to rgb or rgba it treats the variable as the 'red' argument only 🙈 It seems to be a known issue: sass/node-sass#2251

We can work around it by using RGB(var(--variable-name)) instead of lowercase. It's not great, but I guess if it does the job...

[valtism]

@aitchiss Wow, thank you for being so patient and helpful. Your kindness motivated me to spend a couple of hours working out the install errors and getting forem set up locally.

The fix you found works well and everything seems ok now. I wasn't able to get authentication set up locally to create a user and change themes, but changing the variables directly to test on pasted-in content looks good:

[ludwiczakpawel]

I wasn't able to get authentication set up locally to create a user

@valtism you should be able to login with admin@forem.local / password credentials without setting up authentication methods :) (as long as I understand our own docs correctly :))

Also, kudos for installing forem locally!! 🙌

[aitchiss]

This is looking great ✨ I checked each theme in Chrome/Safari/Firefox and it all looks great to me - nice work!

Glad to hear you got Forem up and running locally like Pawel mentioned you should then be able to log in with:

username: admin@forem.local
password: password

If you have any difficulty though, don't hesitate to let us know - hopefully having your environment set up now will be useful for future PRs 😉

[nickytonline]

Thanks for the fix @valtism! 🚀