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.

Sign up for GitHub

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

Jump to bottom

Refactor: Update UNSTABLE_EmptyState according to design #DS-1311 #1481

Merged
merged 4 commits into from
Jun 24, 2024
Merged

Refactor: Update UNSTABLE_EmptyState according to design #DS-1311 #1481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d37001
Refactor(web-twig): Update UNSTABLE_EmptyState according to design #D…
curdaj Jun 19, 2024
8e41425
Refactor(web): Update UNSTABLE_EmptyState according to design #DS-1311
curdaj Jun 19, 2024
41b2ce9
Feat(web-twig): Introduce ActionLayout component #DS-1311
curdaj Jun 19, 2024
6513af0
Feat(web): Introduce ActionLayout component #DS-1311
curdaj Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/web-twig/src/Resources/components/UNSTABLE_ActionLayout/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# UNSTABLE ActionLayout

⚠️ This component is UNSTABLE. It may significantly change at any point in the future.
Please use it with caution.
9 changes: 9 additions & 0 deletions ...eb-twig/src/Resources/components/UNSTABLE_ActionLayout/UNSTABLE_ActionLayout.stories.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{% extends 'layout/plain.html.twig' %}

{% block content %}

<DocsSection title="Default" stackAlignment="stretch">
{% include '@components/UNSTABLE_ActionLayout/stories/ActionLayoutDefault.twig' %}
</DocsSection>

{% endblock %}
6 changes: 6 additions & 0 deletions packages/web-twig/src/Resources/components/UNSTABLE_ActionLayout/UNSTABLE_ActionLayout.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ 'UNSTABLE_ActionLayout' -%}

<div class="{{ _rootClassName }}">
{% block content %}{% endblock %}
</div>
9 changes: 9 additions & 0 deletions ...esources/components/UNSTABLE_ActionLayout/__tests__/UNSTABLE_ActionLayoutSnapshotTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

declare(strict_types=1);

namespace Lmc\SpiritWebTwigBundle\Resources\components\UNSTABLE_EmptyState;

use Lmc\SpiritWebTwigBundle\AbstractComponentSnapshotTest;

class UNSTABLE_ActionLayoutSnapshotTest extends AbstractComponentSnapshotTest {}
5 changes: 5 additions & 0 deletions ...ources/components/UNSTABLE_ActionLayout/__tests__/__fixtures__/UNSTABLE_ActionLayout.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<!-- Render with all props -->
<UNSTABLE_ActionLayout>
<ButtonLink href="#" color="primary">Primary Button</ButtonLink>
<ButtonLink href="#" color="secondary">Secondary Button</ButtonLink>
</UNSTABLE_ActionLayout>
12 changes: 12 additions & 0 deletions ...onents/UNSTABLE_ActionLayout/__tests__/__snapshots__/UNSTABLE_ActionLayout.twig.snap.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<!-- Render with all props --><html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>
</title>
</head>
<body>
<div class="UNSTABLE_ActionLayout">
<a class="Button Button--primary Button--medium" href="#">Primary Button</a> <a class="Button Button--secondary Button--medium" href="#">Secondary Button</a>
</div>
</body>
</html>
39 changes: 39 additions & 0 deletions ...mponents/UNSTABLE_ActionLayout/__tests__/__snapshots__/UNSTABLE_EmptyState.twig.snap.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!DOCTYPE html>
<!-- Render with all props --><html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>
</title>
</head>
<body>
<div class="Stack Stack--hasSpacing UNSTABLE_EmptyState" style="--stack-spacing: var(--spirit-space-700);">
<div class="Stack UNSTABLE_EmptyState__section">
<div style="border: 1px solid #E9E9E9; border-radius: 10px; padding: 12px">
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewbox="0 0 24 24" fill="none" id="0b030f932056ee26f2a78b61d3405eb8" aria-hidden="true">
<path d="M15.5 14H14.71L14.43 13.73C15.63 12.33 16.25 10.42 15.91 8.39002C15.44 5.61002 13.12 3.39002 10.32 3.05002C6.09001 2.53002 2.53002 6.09001 3.05002 10.32C3.39002 13.12 5.61002 15.44 8.39002 15.91C10.42 16.25 12.33 15.63 13.73 14.43L14 14.71V15.5L18.25 19.75C18.66 20.16 19.33 20.16 19.74 19.75C20.15 19.34 20.15 18.67 19.74 18.26L15.5 14ZM9.50002 14C7.01002 14 5.00002 11.99 5.00002 9.50002C5.00002 7.01002 7.01002 5.00002 9.50002 5.00002C11.99 5.00002 14 7.01002 14 9.50002C14 11.99 11.99 14 9.50002 14Z" fill="currentColor">
</path></svg>
</div>
</div>

<div class="Stack Stack--hasSpacing UNSTABLE_EmptyState__section" style="--stack-spacing: var(--spirit-space-500);">
<h2 class="typography-heading-xsmall-text">
Headline
</h2>

<p class="typography-body-medium-text-regular">
In publishing and graphic design, lorem ipsum is common placeholder text used to demonstrate the graphic
elements
</p>
</div>

<div class="Stack UNSTABLE_EmptyState__section">
<div class="UNSTABLE_EmptyState__buttons">
<a class="Button Button--primary Button--medium" href="#">Action</a> <a class="Button Button--secondary Button--medium" href="#">Action</a>
</div>
</div>

<div class="Stack UNSTABLE_EmptyState__section">
<a href="#" class="link-primary">Link to something</a>
</div>
</div>
</body>
</html>
4 changes: 4 additions & 0 deletions .../web-twig/src/Resources/components/UNSTABLE_ActionLayout/stories/ActionLayoutDefault.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<UNSTABLE_ActionLayout>
<ButtonLink href="#" color="primary">Primary Button</ButtonLink>
<ButtonLink href="#" color="secondary">Secondary Button</ButtonLink>
</UNSTABLE_ActionLayout>
16 changes: 0 additions & 16 deletions packages/web-twig/src/Resources/components/UNSTABLE_EmptyState/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,3 @@

⚠️ This component is UNSTABLE. It may significantly change at any point in the future.
Please use it with caution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this can stay here and you can just update the twig structure example.

<EmptyState>
  <EmptyStateSection>
  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1481 (comment)

The `UNSTABLE_EmptyState` component is used to separate content in a layout.

```twig
<UNSTABLE_EmptyState />
```

## API

The components accept [additional attributes][readme-additional-attributes].
If you need more control over the styling of a component, you can use [style props][readme-style-props]
and [escape hatches][readme-escape-hatches].

[readme-additional-attributes]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-react/README.md#additional-attributes
[readme-escape-hatches]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-react/README.md#escape-hatches
[readme-style-props]: https://github.com/lmc-eu/spirit-design-system/blob/main/packages/web-react/README.md#style-props
83 changes: 10 additions & 73 deletions packages/web-twig/src/Resources/components/UNSTABLE_EmptyState/UNSTABLE_EmptyState.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,83 +1,20 @@
{# API #}
{%- set props = props | default([]) -%}
{%- set _buttonPrimaryText = props.buttonPrimaryText | default(null) -%}
{%- set _buttonPrimaryUrl = props.buttonPrimaryUrl | default(null) -%}
{%- set _buttonSecondaryText = props.buttonSecondaryText | default(null) -%}
{%- set _buttonSecondaryUrl = props.buttonSecondaryUrl | default(null) -%}
{%- set _description = props.description -%}
{%- set _headingElementType = props.headingElementType | default('h2') -%}
{%- set _iconName = props.iconName | default(null) -%}
{%- set _linkText = props.linkText | default(null) -%}
{%- set _linkUrl = props.linkUrl | default(null) -%}
{%- set _title = props.title -%}

{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState' -%}
{%- set _headingClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__heading' -%}
{%- set _contentClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__content' -%}
{%- set _buttonsClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__buttons' -%}
{%- set _wrapperClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__wrapper' -%}
{%- set _iconWrapperClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__icon' -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _styleProps.className ] -%}

<div
{{ mainProps(props) }}
{{ styleProp(_styleProps) }}
{{ classProp(_classNames) }}
>
<div class="{{ _wrapperClassName }}">
{% if _iconName %}
<div class="{{ _iconWrapperClassName }}">
<Icon name="{{ _iconName }}"/>
</div>
{% endif %}
<div>
<Heading
elementType="{{ _headingElementType }}"
size="xsmall"
data-test-empty-tile="{{ _headingElementType }}"
UNSTABLE_className="{{ _headingClassName }}"
>
{{ _title }}
</Heading>
<Text color="secondary">{{ _description | raw }}</Text>
</div>
</div>
{% if block('content') is not empty %}
<div class="{{ _contentClassName }}">
{% block content %}{% endblock %}
</div>
{% else %}
{% if _buttonPrimaryUrl or _buttonSecondaryUrl %}
<div class="{{ _buttonsClassName }}">
{% if _buttonPrimaryText and _buttonPrimaryUrl %}
<ButtonLink
href="{{ _buttonPrimaryUrl }}"
color="primary"
>
{{ _buttonPrimaryText }}
</ButtonLink>
{% endif %}
{% if _buttonSecondaryText and _buttonSecondaryUrl %}
<ButtonLink
href="{{ _buttonSecondaryUrl }}"
color="secondary"
>
{{ _buttonSecondaryText }}
</ButtonLink>
{% endif %}
</div>
{% endif %}
{% if _linkUrl and _linkText %}
<Link
href="{{ _linkUrl }}"
color="primary"
>
{{ _linkText }}
</Link>
{% endif %}
{% endif %}
</div>
{%- set _renderedContent %}
{% block content %}{% endblock %}
{% endset -%}

{% embed "@spirit/stack.twig" with { props: props | merge({
UNSAFE_className: _classNames | join(' '),
UNSAFE_style: _styleProps.style,
}) } %}
{% block content %}{{ _renderedContent }}{% endblock %}
{% endembed %}
20 changes: 20 additions & 0 deletions ...ges/web-twig/src/Resources/components/UNSTABLE_EmptyState/UNSTABLE_EmptyStateSection.twig
View file
Open in desktop
curdaj marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{# API #}
{%- set props = props | default([]) -%}

{# Class names #}
{%- set _rootClassName = _spiritClassPrefix ~ 'UNSTABLE_EmptyState__section' -%}

{# Miscellaneous #}
{%- set _styleProps = useStyleProps(props) -%}
{%- set _classNames = [ _rootClassName, _styleProps.className ] -%}

{%- set _renderedContent %}
{% block content %}{% endblock %}
{% endset -%}

{% embed "@spirit/stack.twig" with { props: props | merge({
UNSAFE_className: _classNames | join(' '),
UNSAFE_style: _styleProps.style,
}) } %}
{% block content %}{{ _renderedContent }}{% endblock %}
{% endembed %}
4 changes: 1 addition & 3 deletions ...rc/Resources/components/UNSTABLE_EmptyState/__tests__/UNSTABLE_EmptyStateSnapshotTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,4 @@

use Lmc\SpiritWebTwigBundle\AbstractComponentSnapshotTest;

class UNSTABLE_EmptyStateSnapshotTest extends AbstractComponentSnapshotTest
{
}
class UNSTABLE_EmptyStateSnapshotTest extends AbstractComponentSnapshotTest {}
36 changes: 25 additions & 11 deletions .../Resources/components/UNSTABLE_EmptyState/__tests__/__fixtures__/UNSTABLE_EmptyState.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
<!-- Render with all props -->
<UNSTABLE_EmptyState
buttonPrimaryText="Action"
buttonPrimaryUrl="#"
buttonSecondaryText="Action"
buttonSecondaryUrl="#"
description="Look somewhere else"
iconName="search"
linkText="Link to something"
linkUrl="#"
title="Empty State Title"
/>
<UNSTABLE_EmptyState spacing="space-700">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, nice idea :-) Let's discuss this and there is no right or wrong answer.

From the point of a developer who will be using this, I can now do:

<EmptyState>
  <Heading>Heading</Heading>
  <Text>This is an empty state</Text>
  <Link href="/">Back to homepage</Link>
</EmptyState>

// or 

<EmptyState>
  <EmptyStateContent
        description="This is an empty state"
        heading="Heading"
    />
  <EmptyStateButtons
        buttonPrimaryText="Back to homepage"
        buttonPrimaryUrl="/"
    />
</EmptyState>

// what about the third option? are we letting it go?

<EmptyState
  description="This is an empty state"
   heading="Heading"
   buttonPrimaryText="Back to homepage"
   buttonPrimaryUrl="/"
/>

I am not suggesting that we must do this right now. I am just trying to find the point where we should stop our iterations and merge this and let the developer decide about the ease of use.

There could also be another option, that also makes sense. However the naming of the subcomponent EmptyStateButtons make no sense here:

<EmptyState>
  <EmptyStateContent>
    <Heading>Heading</Heading>
    <Text>This is an empty state</Text>
  </EmptyStateContent>
  <EmptyStateButtons>
    <Link href="/">Back to homepage</Link>
  </EmptyStateButtons>
</EmptyState>

As I wrote at the beginning, there is no right or wrong, just the ease of using the API we fabricate. So maybe just let it be as it is right now and test if the usability is good enough :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to discussion it will be:

<EmptyState>
    <EmptyStateSection>
        <Icon name="icon-name" />
    </EmptyStateSection>
    <EmptyStateSection>
        <Heading>Heading</Heading>
        <Text>This is an empty state</Text>
    </EmptyStateSection>
    <EmptyStateSection>
        <ActionsLayout>
            <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
            <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
        </ActionsLayout>
    </EmptyStateSection>
    <EmptyStateSection>
        <Link href="/">Back to homepage</Link>
    </EmptyStateSection>
</EmptyState>

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are able to make the structure a lot simpler:

<EmptyState>
    <Icon name="icon-name" />
    <Heading>Heading</Heading>
    <Text>This is an empty state</Text>
    <ActionsLayout>
        <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
        <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
    </ActionsLayout>
    <Link href="/">Back to homepage</Link>
</EmptyState>

It just requires something like the following CSS:

.UNSTABLE_EmptyState {
  // …

  max-width: 582px; // if this is the desired limit for the component and not just the demo
  justify-items: center;
}

What do you think? Or did I miss anything?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on the design, there is some space between the sections. That is why we use the subcomponent. Please, look at the Figma file. Maybe we missed something but the offset between the sections is different than between components themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, the problem seems to be in this part:

    <Heading>Heading</Heading>
    <Text>This is an empty state</Text>

because it should be a one block. Technically there are 4 blocks in the component:

  • Image/Illustration
  • Headline+Text
  • Action (buttons)
  • Link

and I need to ensure that they all have a gap between them.

Technically it could be:

<EmptyState>
    <Icon name="icon-name" />
    <div>
        <Heading>Heading</Heading>
        <Text>This is an empty state</Text>
    </div>
    <ActionsLayout>
        <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
        <ButtonLink href="/" color="primary">Back to homepage</ButtonLink>
    </ActionsLayout>
    <Link href="/">Back to homepage</Link>
</EmptyState>

Copy link
Contributor

Choose a reason for hiding this comment

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

@curdaj I'm totally OK with what you suggest. You would use CSS to add a gap among anything inside EmptyState, plus the syntax of the whole EmptyState composition is much simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamkudrna Based on discussion with @literat , we decided to keep EmptyStateSection for now. We consider it as an unstable component and may change it in the future

<UNSTABLE_EmptyStateSection>
<div style="border: 1px solid #E9E9E9; border-radius: 10px; padding: 12px ">
<Icon name="search" />
</div>
</UNSTABLE_EmptyStateSection>
<UNSTABLE_EmptyStateSection spacing="space-500">
<Heading
elementType="h2"
size="xsmall"
>
Headline
</Heading>
<Text color="secondary">In publishing and graphic design, lorem ipsum is common placeholder text used to demonstrate the graphic elements</Text>
</UNSTABLE_EmptyStateSection>
<UNSTABLE_EmptyStateSection>
<UNSTABLE_ActionLayout>
<ButtonLink color="primary" href="#">Action</ButtonLink>
<ButtonLink color="secondary" href="#">Action</ButtonLink>
</UNSTABLE_ActionLayout>
</UNSTABLE_EmptyStateSection>
<UNSTABLE_EmptyStateSection>
<Link href="#">Link to something</Link>
</UNSTABLE_EmptyStateSection>
</UNSTABLE_EmptyState>
32 changes: 19 additions & 13 deletions ...components/UNSTABLE_EmptyState/__tests__/__snapshots__/UNSTABLE_EmptyState.twig.snap.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,35 @@
</title>
</head>
<body>
<div class="UNSTABLE_EmptyState">
<div class="UNSTABLE_EmptyState__wrapper">
<div class="UNSTABLE_EmptyState__icon">
<div class="Stack Stack--hasSpacing UNSTABLE_EmptyState" style="--stack-spacing: var(--spirit-space-700);">
<div class="Stack UNSTABLE_EmptyState__section">
<div style="border: 1px solid #E9E9E9; border-radius: 10px; padding: 12px">
<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewbox="0 0 24 24" fill="none" id="0b030f932056ee26f2a78b61d3405eb8" aria-hidden="true">
<path d="M15.5 14H14.71L14.43 13.73C15.63 12.33 16.25 10.42 15.91 8.39002C15.44 5.61002 13.12 3.39002 10.32 3.05002C6.09001 2.53002 2.53002 6.09001 3.05002 10.32C3.39002 13.12 5.61002 15.44 8.39002 15.91C10.42 16.25 12.33 15.63 13.73 14.43L14 14.71V15.5L18.25 19.75C18.66 20.16 19.33 20.16 19.74 19.75C20.15 19.34 20.15 18.67 19.74 18.26L15.5 14ZM9.50002 14C7.01002 14 5.00002 11.99 5.00002 9.50002C5.00002 7.01002 7.01002 5.00002 9.50002 5.00002C11.99 5.00002 14 7.01002 14 9.50002C14 11.99 11.99 14 9.50002 14Z" fill="currentColor">
</path></svg>
</div>
</div>

<div class="Stack Stack--hasSpacing UNSTABLE_EmptyState__section" style="--stack-spacing: var(--spirit-space-500);">
<h2 class="typography-heading-xsmall-text">
Headline
</h2>

<div>
<h2 data-test-empty-tile="h2" class="typography-heading-xsmall-text">
Empty State Title
</h2>
<p class="typography-body-medium-text-regular">
In publishing and graphic design, lorem ipsum is common placeholder text used to demonstrate the graphic
elements
</p>
</div>

<p class="typography-body-medium-text-regular">
Look somewhere else
</p>
<div class="Stack UNSTABLE_EmptyState__section">
<div class="UNSTABLE_ActionLayout">
<a class="Button Button--primary Button--medium" href="#">Action</a> <a class="Button Button--secondary Button--medium" href="#">Action</a>
</div>
</div>

<div class="UNSTABLE_EmptyState__buttons">
<a class="Button Button--primary Button--medium" href="#">Action</a> <a class="Button Button--secondary Button--medium" href="#">Action</a>
<div class="Stack UNSTABLE_EmptyState__section">
<a href="#" class="link-primary">Link to something</a>
</div>
<a href="#" class="link-primary">Link to something</a>
</div>
</body>
</html>
Loading
Loading