1845151 - Remove all code associated with the message-bar component

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, task)

Description

[Hanna Jones [:hjones]]

Once all existing uses of message-bar have been replaced with moz-message-bar we should remove all the code associated with the older element as it will no longer be needed. It's likely we'll have to do this when we remove the last usage, as I expect we'll get a failure in the browser_all_files_referenced.js, but I'm filing this to make sure we do all the necessary cleanup. Specifically we should:

• remove message-bar.js: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/message-bar.js
• remove message-bar.css: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/message-bar.css
• remove the message-bar story: https://searchfox.org/mozilla-central/source/browser/components/storybook/stories/message-bar.stories.mjs
• rename the moz-message-bar story to "Message Bar" since there will no longer be any ambiguity around which is which

Comment 1

[Ganna]

Attachment: Bug 1845151 - Remove all code associated with the message-bar component r=hjones

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d137dadab8f
Remove all code associated with the message-bar component r=hjones,desktop-theme-reviewers,reusable-components-reviewers,extension-reviewers,robwu

Comment 3

[Cristian Tuns]

Backed out for causing bc failures in browser_parsable_css.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | custom property --green-50 is not referenced -

Comment 4

[Ganna]

(In reply to Cristian Tuns from comment #3)

Backed out for causing bc failures in browser_parsable_css.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | custom property --green-50 is not referenced -

I wonder why we get this error when we use this variable here: https://searchfox.org/mozilla-central/source/toolkit/components/aboutwebauthn/content/aboutWebauthn.css#35

Comment 5

[Hanna Jones [:hjones]]

(In reply to Ganna from comment #4)

(In reply to Cristian Tuns from comment #3)

Backed out for causing bc failures in browser_parsable_css.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | custom property --green-50 is not referenced -

I wonder why we get this error when we use this variable here: https://searchfox.org/mozilla-central/source/toolkit/components/aboutwebauthn/content/aboutWebauthn.css#35

Dug into the code a bit and I'm pretty sure the issue is that the aboutWebauthn code is Mac/Linux only, so it doesn't get loaded on Windows - see here

That tracks with what we're seeing, since the test is only failing for Windows builds.

I think we have three options here:

• Replace the --green-50 usage with one of our design tokens - we don't have anything super close, but maybe --color-green-30 would be alright (ideally mapped to a more semantic variable based on how it's being used). It's possible we have a green variable that is closer that we just haven't had a reason to use yet, in which case we could add that value to tokens-shared.css then use it here; :jules probably has more info about our colors. This will add a bit of scope, but is my personal preferred fix.
• Wrap the --green-50 definition in a platform specific media query - something like @media not (-moz-platform: win) (ideally with a comment to explain why)
• Add a rule to the ignoreList in the test file

Comment 6

[Ganna]

(In reply to Hanna Jones [:hjones] from comment #5)
I tried second option (wrapping the variable in a platform specific media query), but the test still fails. So I feel like the first option is the best. But I am not sure which semantic variable name we can choose for this color. --green-50 (#30e60b) is used as a success background-color in aboutWebauthn.css. We have a --background-color-success variable in https://searchfox.org/mozilla-central/source/toolkit/themes/shared/design-system/tokens-shared.css#17, but that's a very different shade of green.

Comment 7

[Jules Simplicio [:jules]]

Hey y'all. So the existing green 50 variable is a rather bright lime green that our branding has moved away from. It'd be totally fine to go with our new token for success, in my personal opinion. Thanks for working this out Anna and for suggesting the token replacement Hanna.

Comment 8

[Ganna]

(In reply to Jules Simplicio [:jules] from comment #7)

Hey y'all. So the existing green 50 variable is a rather bright lime green that our branding has moved away from. It'd be totally fine to go with our new token for success, in my personal opinion. Thanks for working this out Anna and for suggesting the token replacement Hanna.

Thank you for the reply. But in this case, should we also replace error bacground color here https://searchfox.org/mozilla-central/source/toolkit/components/aboutwebauthn/content/aboutWebauthn.css#39 with --background-color-critical token?

Comment 9

[Jules Simplicio [:jules]]

That's a great point. I'd probably file a separate bug for that since it's unrelated to this problem we're having. What do you think? But also...I wonder what this UI is and if it could use a message bar instead? ha

Comment 10

[Ganna]

(In reply to Jules Simplicio [:jules] from comment #9)

That's a great point. I'd probably file a separate bug for that since it's unrelated to this problem we're having. What do you think? But also...I wonder what this UI is and if it could use a message bar instead? ha

That's a great question. I can't eccess about:webauthn page to check that, as it is for Linux and Mac, if I am not mistaken. But it definately looks like some sort of message for a user, which is the main purpose of the message bar)

Comment 11

[Jules Simplicio [:jules]]

What do you think, Hanna?

Comment 12

[Hanna Jones [:hjones]]

Attachment: before and after color change

(In reply to Jules Simplicio [:jules] from comment #11)

What do you think, Hanna?

I did a little before/after test by forcing the message to show. I think the color change is definitely an improvement, and we should also get a bug on file to use moz-message-bar there. I imagine this isn't a super high traffic page, but it should be an easy improvement to make :)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8d490edd8c3
Remove all code associated with the message-bar component r=hjones,desktop-theme-reviewers,reusable-components-reviewers,extension-reviewers,robwu

Comment 14

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/f8d490edd8c3