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

feat: Profile card. #1039

Merged
merged 9 commits into from
Oct 22, 2021
Merged

feat: Profile card. #1039

merged 9 commits into from
Oct 22, 2021

Conversation

mturoci
Copy link
Collaborator

@mturoci mturoci commented Oct 4, 2021

image
image
image

Proposed API

/** Create a profile card to display information about a user. */
interface State {
  /** The card's title, displayed under the main image. */
  title: S
  /** The card's subtitle, displayed under the title. */
  subtitle?: S
  /** 
   * The card’s image, either a base64-encoded image, a path to an image hosted externally (starting with `https://` or `http://`)
   * or a path to an image hosted on the Wave daemon (starting with `/`).
. */
  image?: S
  /** 
   * The avatar’s image, either a base64-encoded image, a path to an image hosted externally (starting with `https://` or `http://`)
   * or a path to an image hosted on the Wave daemon (starting with `/`).
. */
  profile_image?: S
  /** Initials, if `profile_image` is not specified. */
  initials?: S
  /** Components in this card displayed below toolbar / image. */
  items?: Component[]
}

Further questions: #1033 (comment)

Set min-height for background image to 150px.

Design questions

  • The proposed 40px profile image size was too small considering it should be one of the more prominent elements of this card. Went with 100px instead. Let me know if that's fine.
  • Same with title, 16px was not big enough, went with 20px.
  • Centered profile image on the edge of the background image. The design proposal alignment seemed random.
  • The design doesn't account for :hover state (see pic below), thus spacing does not match the one in design. What to do with this?
    image
  • Profile pic border looks odd with light theme. Need to either pick different color or remove it completely.

Closes #1033

@mturoci mturoci requested a review from lo5 as a code owner October 4, 2021 05:25
@mturoci mturoci changed the title feat/issue 1033 Profile card Oct 4, 2021
@mturoci mturoci changed the title Profile card feat: Profile card. Oct 4, 2021
@shihan007
Copy link

@mturoci in light theme name and profile pic space seems reduced, dark theme looks good. For pic border color which color variable did you use?

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 4, 2021

in light theme name and profile pic space seems reduced

my bad, accidentally took a screenshot of wrong code

image
image

For pic border color which color variable did you use?

neutralPrimary as specified in designs

@shihan007
Copy link

@mturoci For border color lets go with neutralTertiary. Hard to find a variable that matches with each theme.
Fine with the pro pic size and font size.

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 4, 2021

Hard to find a variable that matches with each theme.

Feel free to come up with your own color computation if you feel like the Fluent one is too hard to work with.

Fine with the pro pic size and font size.

cool, thanks

What about the rest of the points?

@shihan007
Copy link

@mturoci since we are using most icons in here we can go with 24px button as we did in the other cards.

@lo5
Copy link
Member

lo5 commented Oct 4, 2021

@mturoci - How about this:

/** Create a profile card to display information about a user. */
interface State {
  /** The persona represented by this card. */
  persona: Persona
  /** 
   * The card’s background image, either a base64-encoded image, a path to an image hosted externally (starting with `https://` or `http://`)
   * or a path to an image hosted on the Wave daemon (starting with `/`).
. */
  image?: S
  /** Components displayed inside this card. */
  items?: Component[]
}

Note:

  • Can rename to persona_card (since we've already shipped ui.persona()).
  • caption, size, and name from persona can be ignored by this card's implementation.

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 5, 2021

@lo5 yes, this was my first go to, but then realized that ui.persona returns Component, same problem as #1007 (comment).

Can rename to persona_card (since we've already shipped ui.persona())

Good point, will do.

@lo5
Copy link
Member

lo5 commented Oct 5, 2021

yes, this was my first go to, but then realized that ui.persona returns Component,

Should be OK, since call sites are not affected:

card = ui.persona_card(persona=ui.persona(...), image='...', items=[...])

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 6, 2021

Should be OK, since call sites are not affected:

Not sure what you mean. This is what I get:

image

with code:

from h2o_wave import site, ui

page = site['/demo']

image = 'https://images.pexels.com/photos/3225517/pexels-photo-3225517.jpeg?auto=compress&cs=tinysrgb&dpr=2&h=750&w=1260' # noqa
page.add('example', ui.profile_card(
    box='1 1 3 5',
    persona=ui.persona(title='John Doe', subtitle='Data Scientist', caption='Online', size='xs', image=image),
    commands=[
        ui.command(name='new', label='New', icon='Add', items=[
            ui.command(name='email', label='Email Message', icon='Mail'),
            ui.command(name='calendar', label='Calendar Event', icon='Calendar'),
        ]),
        ui.command(name='upload', label='Upload', icon='Upload'),
        ui.command(name='share', label='Share', icon='Share'),
        ui.command(name='download', label='Download', icon='Download'),
    ],
    items=[
        ui.inline(justify='center', items=[
            ui.button(name='btn1', label='Button 1'),
            ui.button(name='btn2', label='Button 2'),
            ui.button(name='btn3', label='Button 3'),
        ]),
    ]
))

page.save()

Maybe we could adjust wavegen in a way that it would decide whether to return wrapped persona within component or just a simple persona based on some kind of Python reflection that would search the stacktrace for form_card occurence?

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 8, 2021

Something like this (naive example):

def persona(
        title: str,
        subtitle: Optional[str] = None,
        caption: Optional[str] = None,
        size: Optional[str] = None,
        image: Optional[str] = None,
        initials: Optional[str] = None,
        initials_color: Optional[str] = None,
        name: Optional[str] = None,
) -> Union[Component, Persona]:
    """Create an individual's persona or avatar, a visual representation of a person across products.
    Can be used to display an individual's avatar (or a composition of the person’s initials on a background color), their name or identification, and online status.

    Args:
        title: Primary text, displayed next to the persona coin.
        subtitle: Secondary text, displayed under the title.
        caption: Tertiary text, displayed under the subtitle. Only visible for sizes >= 'm'.
        size: The size of the persona coin. Defaults to 'm'. One of 'xl', 'l', 'm', 's', 'xs'. See enum h2o_wave.ui.PersonaSize.
        image: Image, URL or base64-encoded (`data:image/png;base64,???`).
        initials: Initials, if `image` is not specified.
        initials_color: Initials background color (CSS-compatible string).
        name: An identifying name for this component.
    Returns:
        A `h2o_wave.types.Persona` instance.
    """
    for stack in inspect.stack():
        if 'form_card' in stack.function:
            return Component(persona=Persona(
                title,
                subtitle,
                caption,
                size,
                image,
                initials,
                initials_color,
                name,
            ))
    return Persona(
        title,
        subtitle,
        caption,
        size,
        image,
        initials,
        initials_color,
        name,
    )

@lo5
Copy link
Member

lo5 commented Oct 8, 2021

By "call sites are not affected", I meant that it makes no difference to the following line whether ui.persona() returns a Component or a Persona.

card = ui.persona_card(persona=ui.persona(...), image='...', items=[...])

This is not special to persona_card or persona. All the existing components work this way (e.g. a button inside buttons).

Maybe we could adjust wavegen in a way that it would decide whether to return wrapped persona within component or just a simple persona based on some kind of Python reflection that would search the stacktrace for form_card occurence?

It's not urgent right now, and the persona card will work just fine without any modifications to wavegen.

A Persona is modeled as a variant of Component, not a subclass (Component is a union type). I understand it's possible to pass in the wrong kind of component, but Python doesn't provide safety at runtime anyway. Union[Component, Persona] wouldn't be a proper solution to this, because it doesn't solve the primary problem of preventing the wrong kind of component from being passed in.

So I would recommend proceeding without changes to wavegen.

@mturoci
Copy link
Collaborator Author

mturoci commented Oct 11, 2021

After discussion with @lo5, the final API should be

/** Create a profile card to display information about a user. */
interface State {
  /** The persona represented by this card. */
  persona: Component
  /** 
   * The card’s background image, either a base64-encoded image, a path to an image hosted externally (starting with `https://` or `http://`)
   * or a path to an image hosted on the Wave daemon (starting with `/`).
. */
  image?: S
  /** Components displayed inside this card. */
  items?: Component[]
}

@mturoci mturoci merged commit 8ca32c5 into master Oct 22, 2021
@mturoci mturoci deleted the feat/issue-1033 branch October 22, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Profile card
3 participants