V1.5 Alert

[yangchristina]

No description provided.

[raineorshine]

Thanks for your submission!

The main Alert component is represented by https://github.com/cybersemics/em/blob/main/src/components/Popup.tsx. https://github.com/cybersemics/em/blob/main/src/components/Alert.tsx is a HOC that mainly just handles fade in/out. By circumventing Popup.tsx we're losing some key functionality:

• Cross-browser support for position: fixed
• Swipe-to-dismiss functionality
• Multicursor support
• Import Files special handling.

The goal is to re-style the alert without losing any of the existing functionality. I would suggest duplicating https://github.com/cybersemics/em/blob/main/src/components/Popup.tsx and using that as a starting point. The only other component that uses it is

em/src/components/CommandPalette.tsx

Line 469 in 42b1ef5

<Popup

, and that will be re-styled as well so we could make them separate components to decouple them.

Let me know if this makes sense!

[raineorshine]

Could we add an ✗ that appears in the upper right corner on hover to dismiss the alert? I realize the mockup did not account for the close functionality on desktop.

I'm okay if this is + time.

[yangchristina]

Yes I can add it.

[yangchristina]

Thanks for your submission!

The main Alert component is represented by https://github.com/cybersemics/em/blob/main/src/components/Popup.tsx. https://github.com/cybersemics/em/blob/main/src/components/Alert.tsx is a HOC that mainly just handles fade in/out. By circumventing Popup.tsx we're losing some key functionality:

• Cross-browser support for position: fixed
• Swipe-to-dismiss functionality
• Multicursor support
• Import Files special handling.

The goal is to re-style the alert without losing any of the existing functionality. I would suggest duplicating https://github.com/cybersemics/em/blob/main/src/components/Popup.tsx and using that as a starting point. The only other component that uses it is

em/src/components/CommandPalette.tsx

Line 469 in 42b1ef5

<Popup

, and that will be re-styled as well so we could make them separate components to decouple them.
Let me know if this makes sense!

Should font size and padding still follow these?

  const fontSize = useSelector(state => state.fontSize)
  const padding = useSelector(state => state.fontSize / 2 + 2)

[yangchristina]

I'll update the puppeteer snapshot once the styling has been confirmed.

[raineorshine]

Let's go with scaling the fontSize and em units for margin and padding. If we use scale on the Alert component but not other components, I think it's going to cause small aspects like borders to be inconsistent.

[yangchristina]

Let's go with scaling the fontSize and em units for margin and padding. If we use scale on the Alert component but not other components, I think it's going to cause small aspects like borders to be inconsistent.

Sounds good!

[trevinhofmann]

Thanks for the PR, @yangchristina!

Quick note: Please include a PR description explaining the goals of the change. I have the design document for these UI/UX changes, but it would still be nice to know a bit more about which particular parts the PR is changing.

---

1. The "x" button does not close the alert

The "x" button appears correctly on hover (although it's a bit small, IMO), but clicking it does not dismiss the alert popup.

Screen.Recording.2025-01-06.at.5.45.20.PM.mov

---

2. Confusing cursor pointer on icons

This might be intentional and a non-issue, but it was confusing to me that the cursor changes to a pointer when hovering over the icon. Normally, the pointer is reserve for clickable items that cause an action, but this icon is not clickable.

Screen.Recording.2025-01-06.at.5.46.31.PM.mov

---

On mobile, I have a few questions that I would just like to clarify (cc @raineorshine).

A) Should the alert be centered, or is it intended to be on the right side of the screen? I don't see a mobile design for the alert in Figma.
B) Should the alert still fade out / dismiss while I'm actively dragging it?
C) Should the alert be dismissed by dragging it up/down, or left/right? I'm fine with either, but my initial "training" as a user made me assume that it would be dismissed by dragging it left/right.

alert.mp4

[yangchristina]

@trevinhofmann Here's the figma, it is supposed to centred.

There aren't mocks for the 'x', so I didn't know how large to make it. I can make it larger. @raineorshine what size would be good?

[yangchristina]

@raineorshine for the 'x' button, should it only show when alert?.showCloseLink is true?

[raineorshine]

A) Should the alert be centered, or is it intended to be on the right side of the screen? I don't see a mobile design for the alert in Figma.

Centered

B) Should the alert still fade out / dismiss while I'm actively dragging it?

I don't think it needs to fade out while actively dragging it. I'm basing this on the current behavior in main.

C) Should the alert be dismissed by dragging it up/down, or left/right? I'm fine with either, but my initial "training" as a user made me assume that it would be dismissed by dragging it left/right.

Since it shows up at the bottom of the screen, my tendency is to dismiss it by dragging it down. Let's stick with down for now, but I definitely understand the left/right tendency as well. I'd love to do some user testing on this to see what is most intuitive for most people.

There aren't mocks for the 'x', so I didn't know how large to make it. I can make it larger. @raineorshine what size would be good?

We can follow the mockup for Tip: https://www.figma.com/design/g0osrpkb5dhY0qPxW9mWjB/Em-UX?node-id=227-886&t=qhUgnmnLKgPDUViF-4

A couple points:

• There is additional padding to make room for the ✗ button. This should be applied to both sides for balance.
• The tappable area should extend slightly beyond the visible ✗, just like in main.

[yangchristina]

Does this look around right? The icon in the mocks is thinner than the text, but CloseButton uses text instead of an svg icon.

Spacing wise, using text is less exact for vertical padding.

@raineorshine what do you think of switching the close button to the svg icon in the figma mocks instead of the text character?

[raineorshine]

That does look close to the mockup, although I didn't anticipate how it would look with a single-line alert. I don't think the vertical centering looks great for a close button, which is expected to be in the upper right.

I was thinking more like this:

Or we could move it to the corner in a circle, which I think looks a bit better:

I could ask Ahmad to do a mockup of the latter so you have more exact proportions to work with.

I'm fine with using SVG for it if that helps.

[yangchristina]

I don't think there's a simple way to move the alert up when the keyboard opens. However I can make it so that if the keyboard is open, you can see the alert. However if the alert was there before the keyboard opened, it will get covered once the keyboard opens. Is this okay @raineorshine?

Screen.Recording.2025-01-26.at.5.20.26.PM.mov

[yangchristina]

@raineorshine I've also updated the close button svg. Does this look right?

[raineorshine]

I don't think there's a simple way to move the alert up when the keyboard opens. However I can make it so that if the keyboard is open, you can see the alert. However if the alert was there before the keyboard opened, it will get covered once the keyboard opens. Is this okay @raineorshine?

The alert is also used for ongoing states such as drag-and-drop, multicursor, import progress, etc. I'm afraid that if the keyboard covers the alert, the user won't have this feedback.

Another problem is the inverse: If the user triggers an alert while the keyboard is up, and then closes the keyboard, the alert is left floating in the middle of the screen.

In both cases we do need a true position: fixed at the bottom of the viewport, above the keyboard.

Instead of using window.visualViewport, which is non-reactive (and buggy), my suggestion is to use our viewportStore.

const { innerHeight, virtualKeyboardHeight } = viewportStore.useState()

@raineorshine I've also updated the close button svg. Does this look right?

I haven't tested it at multiple font sizes, but looks great so far! A few suggestions:

• I think the padding of the close button in the mockup looks a little small. I bumped the width from 20 to 24 in the mockup. Could you increase the padding accordingly?
• Let's make it desktop only. Mobile fires a hover event on click ("click intent") which we don't want in this case.
• Expand the click area about 10px on all sides to make it easier to click. I typically do this with a combination of padding and negative margin.
• Ensure that the expanded click area is available at all times, not just on hover. Otherwise hovering a few pixels above the upper-right corner where the ✗ should appear won't do anything initially.

[trevinhofmann]

Hi, @yangchristina! I will delay reviewing for now until you have a chance to address the latest feedback from @raineorshine. Please let me know when this is ready again for review, and I'll be happy to have another look. Thank you!

[yangchristina]

@raineorshine viewportStore doesn't update when keyboard is up, so it doesn't fix this. I can't find a solution to this (tried touchmove too but doesn't work)
(using position: fixed bottom will not show when keyboard comes up, so I need to to add the keyboard height when the keyboard comes up. To do this I need some sort of listener for when the keyboard comes up)

[raineorshine]

Thanks for the update. I didn't realize viewportStore was not updated when the virtual keyboard activated. We should be able to update it in response to the selectionchange event, when selection.isActive() changes. Then innerHeight will be accurate.

If that doesn't work, or if you need additional guidance, @trevinhofmann can work with you on a solution. Thanks!

[yangchristina]

@raineorshine actually it kinda works sometimes? There was one point where the alert was actually working, but now it's back to not really working. (didn't make any code changes, just seems really inconsistent)

Screen.Recording.2025-01-30.at.12.35.13.AM.mov

[yangchristina]

Here's another example of it being really inconsistent. I'm not sure if it's just a problem with the simulator. From the logs, viewportStore does actually update correctly, but changes don't always show, or show inconsistently

Screen.Recording.2025-01-30.at.12.39.15.AM.mov

[raineorshine]

That is strange. Some ideas:

1. Confirm that XCode Simulator does not behave differently from physical phone.
2. Confirm viewportStore.update is called whenever the keyboard changes.
3. Confirm Alert is re-rendered correctly when the window size changes.

There is also a question of timing. We don't want the Alert position to be delayed, and it's probably not possible for it to smoothly animate down with the keyboard. Our best bet is probably to have the Alert snap to the bottom as soon as the keyboard is closed. The keyboard should cover the alert while animating out of view. Then the Alert will be in its place at the bottom of the screen.

However, it's not really possible to design the correct behavior when you are getting inconsistent behavior, so I would focus on understanding the cause of the inconsistency first. There is usually another variable we are not aware of that is affecting the behavior.

Hope that helps!

[yangchristina]

@raineorshine I don't really have a way of running this on an actual phone. (I don't use an iphone and even if I did I don't know how I'd connect that phone to em's localhost)

[raineorshine]

No problem, one of us can verify that. It's unlikely that it's the problem, but it doesn't hurt to verify base assumptions when inconsistent behavior is observed.