Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated the Toast appearance and positioning #5

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

knutsynstad
Copy link
Collaborator

@knutsynstad knutsynstad commented Nov 2, 2023

Changes:

  • Changes appearance of toast
  • Changed position of toast
  • Shortened toast message
  • Minor variable cleanup

Before:

Screenshot 2023-11-02 at 2 34 13 PM Screenshot 2023-11-02 at 2 34 06 PM

After:

Screenshot 2023-11-02 at 2 33 18 PM Screenshot 2023-11-02 at 2 33 27 PM

@@ -6,6 +6,7 @@ import {unsafeHTML} from 'lit/directives/unsafe-html.js'
import addOutline from '../assets/icons/add-outline.svg'
import caretDownOutline from '../assets/icons/caret-down-outline.svg'
import caretUpOutline from '../assets/icons/caret-up-outline.svg'
import checkmarkFill from '../assets/icons/checkmark-fill.svg'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice touch 👍

@@ -99,7 +99,7 @@ export class PlayPenHeader extends LitElement {
appearance="orangered"
size="small"
icon="share-new-outline"
title="Copy Program URL to Clipboard"
title="Copy URL to Clipboard"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔕 This makes sense to me. Just some background: the original intent here was to indicate that the program / pen is fully represented in the URL which users may not realize. Product banned pen terminology and calling everything a playground is ambiguous.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I mainly just want it as short as possible. But also not sure if we've used the term "program" elsewhere in the UI.

As for the overall naming, my mental model is:
Play = Product name
Playground = Product category

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is:

  • Play: product
  • Playground: execution environment or sandbox; there may be multiple of these on a page like the docs and they each have their own independent state.
  • Pen: program and the thing you share.

}

protected override render() {
return html`<slot></slot>`
return html`<play-icon size="24px" icon="checkmark-fill"></play-icon
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔕 I think this icon should be outside the toast so we can use it for things besides confirmation but not a big deal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Making the icon a fixed part of the toast makes the component overall less versatile.

I don't think we'll use toasts for error messages, given it's short display duration, but it's certainly possible for messages of more neutral intent that doesn't warrant the icon.

transition-duration: 0.1s;
transition-timing-function: ease-out;

/* RPL/Body Regular/14-BodyReg */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔕 Suggest moving all this stuff into a CSS variable or inheriting.

@knutsynstad knutsynstad merged commit 71128f8 into main Nov 2, 2023
1 check passed
@knutsynstad knutsynstad deleted the toast-appearance branch November 2, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants