1809722 - Add support for dismiss button in spotlight split layout

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Punam Dahiya [:pdahiya]]

With MR onboarding , we are using split position for new and continuous onboarding. Scope of this bug is to update dismiss button logic to support both default center and split position.

https://searchfox.org/mozilla-central/rev/fb9a504ca73529fa550efe488db2a012a4bf5169/browser/components/newtab/content-src/aboutwelcome/components/MultiStageProtonScreen.jsx#403

https://searchfox.org/mozilla-central/rev/fb9a504ca73529fa550efe488db2a012a4bf5169/browser/components/newtab/content-src/aboutwelcome/components/MultiStageAboutWelcome.jsx#357

This enhancement can be possibly used in Window vs Tab Modal experiment currently scheduled for Fx110

Comment 1

[Punam Dahiya [:pdahiya]]

Attachment: Spotlight dismiss button in position center layout

Comment 2

[Punam Dahiya [:pdahiya]]

Attachment: Spotlight dismiss button in split layout

Comment 3

[Shane Hughes [:aminomancer]]

Is there a prototype on figma?

Comment 4

[Punam Dahiya [:pdahiya]]

(In reply to Shane Hughes [:aminomancer] from comment #3)

Is there a prototype on figma?

Thanks Shane , here is figma from window modal treatment A.2 branch showing modal with dismiss button

https://www.figma.com/file/ZGfPtwxqki3dWLB0c1CJmY/Fall%2FWinter-2022-Experiments?node-id=3932%3A51010&t=NpRLXDFEZ4IUWNgK-1

Comment 5

[Shane Hughes [:aminomancer]]

Sounds good, I'll take a crack at it

Comment 6

[Shane Hughes [:aminomancer]]

Attachment: Bug 1809722 - Add dismiss button to spotlight. r=pdahiya!

Comment 7

[Shane Hughes [:aminomancer]]

Attachment: Normal width

Normal width

Comment 8

[Shane Hughes [:aminomancer]]

Attachment: Smaller width

Hey Gabrielle, can you confirm if these button placements are good? Thanks!

Comment 9

[Gabrielle Lussier [:glussier]]

(In reply to Shane Hughes [:aminomancer] from comment #8)

Created attachment 9314599 [details]
Smaller width

Hey Gabrielle, can you confirm if these button placements are good? Thanks!

My bad, forgot about the responsive layout 😅 would it be possible to have the X on top of the header image? I threw together a mockup in figma, as well as how the the non-MR spotlight modals look with the X.

https://www.figma.com/file/ZGfPtwxqki3dWLB0c1CJmY/Fall%2FWinter-2022-Experiments?node-id=4534%3A30490&t=4nP5OR4uHe46l7GW-1

Comment 10

[Gabrielle Lussier [:glussier]]

Attachment: DarkmodemodalX.png

Comment 11

[Shane Hughes [:aminomancer]]

No worries thanks for letting me know. Looks good, we'll make it happen

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fcb7c2dd6ff
Add dismiss button to spotlight. r=pdahiya

Comment 13

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4fcb7c2dd6ff

Comment 14

[Shane Hughes [:aminomancer]]

Are we planning to show a dismiss button on every screen? Or will it disappear after the first screen?

If the dismiss button is going to disappear between screens, we want it to transition smoothly out of the screen. But if it's going to stay present on every screen throughout the whole flow, then we want it to sit still, like the logo does.

Unfortunately it's not possible to support both with pure CSS. If we support dismiss button transitions betweeen screens, then it's going to slide/fade in between every screen, even if both screens have a dismiss button. And if we don't support transitions between screens, then of course all messages with dismiss buttons on any screen would have to have a dismiss button on ALL screens.

So if we want to support both of these possibilities (having a dismiss button on some screens but not all; or having a dismiss button on every screen), we'll need to add some JS or change how the ref keying works, so the dismiss button on Screen 1 knows if it's going to be present on Screen 2. It would be neatest if we just had a convention one way or the other. Right now I think we only use dismiss buttons on the first screen. But for the onboarding flow, it seems kinda weird for the dismiss button to disappear after the first screen, since from the user's point of view the overall screen template remains the same.

Comment 15

[Vtay]

I would defer to @glussier on this.

I think from an a11y POV, we would want a dismiss button on every screen. I personally would prefer having a dismiss button on every screen from 1) user choice 2) a11y POV.

Comment 16

[Gabrielle Lussier [:glussier]]

Yep, the intent is for the dismiss to persist on all steps for this variant. My bad, I thought this was documented somewhere. I've updated the Figma to show the whole flow with the dismiss button in the top right. And since it persists I'm cool with it being treated like the logo and not being animated (unless there's another issue here I'm not understanding.) thanks!!

Comment 17

[Shane Hughes [:aminomancer]]

(In reply to Gabrielle Lussier [:glussier] from comment #16)

Yep, the intent is for the dismiss to persist on all steps for this variant. My bad, I thought this was documented somewhere. I've updated the Figma to show the whole flow with the dismiss button in the top right. And since it persists I'm cool with it being treated like the logo and not being animated (unless there's another issue here I'm not understanding.) thanks!!

Thanks! I probably didn't explain the dilemma very well. Do we know if there are going to be any variants that don't have a dismiss button? Right now, we only show the dismiss button on the first screen (same as the Sign In button) because if we showed it on every screen, it would unnecessarily slide in and out between every screen. This sounds a bit odd but I'll try to explain why - Between every screen, all the elements are removed and then recreated. So if they are set up to have any transitions between screens, those transitions are going to trigger, even if the same element exists on both screens. If Screen 1 has a dismiss button but Screen 2 has no dismiss button, that works great, and the dismiss button slides down and disappears for good. But if both screens have a dismiss button, what happens is Screen 1's dismiss button slides out, then Screen 2's dismiss button slides into the same place. So it looks kinda weird, as the dismiss button didn't need to go anywhere.

But the only solutions are 1) to use javascript to trigger the animations more intelligently, which adds complexity that we would probably avoid until it's necessary, or 2) to just not trigger those sliding animations - that way, although the dismiss button is removed and recreated between every screen, we aren't animating it, so nothing changes visually. That said, there are actually 4 kinds of transitions/animations. One kind of transition plays when the first screen appears. Another kind plays when the final screen ends. And then we have transition in and transition out between the screens. And it's those transitions between screens that cause this problem. So there's no problem with having the dismiss button smoothly transition in the first screen and out in the final screen (we can continue to support those first and final transitions on the dismiss button either way).

But if we don't trigger the between-screen animations, it means that any time there's a dismiss button on Screen 1 but not on Screen 2, the dismiss button will just abruptly disappear in between those screens without any smooth transition. That's what we do with the logo, but it's okay because the logo always appears on either every screen or no screen. Whereas the dismiss button currently appears on only the first screen. So I think we could exempt the dismiss button from these between-screen animations, but only if we make sure the dismiss button appears on every screen of every message like the logo does. If we want to support messages where it appears on every screen and messages where it appears on just one screen, then we need to take the more complex route. Or if we want to support messages where it appears on any two consecutive screens but not on all screens, that will have the same requirements.

I'm feeling partial to showing the dismiss button on every screen, but idk how much confidence we have that there won't be messages (say, for experiments) where we only want the dismiss button to appear on one screen or on a few. And if there is even just one message variant like that, then we'd need to account for it by adding some logic to trigger the transitions dynamically.

Comment 18

[Vtay]

I'm going to tag @najla with regards to future intentions

Comment 19

[nbulous]

What you've decided seems reasonable to me. I can't think of a scenario where we don't show the dismiss button on every screen at this point in time.