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

fix: Hide CoreText in preview if empty value. WF-20 #482

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

madeindjs
Copy link
Collaborator

@madeindjs madeindjs commented Jun 29, 2024

Simply hide the CoreText using v-if in preview mode if the value is emptyText. We still display the component in edit mode.

Screencast.from.2024-06-29.14-37-01.webm

@madeindjs madeindjs marked this pull request as draft June 29, 2024 18:24
@madeindjs madeindjs force-pushed the WF-20 branch 2 times, most recently from 89596aa to 7f39a42 Compare June 29, 2024 19:16
@madeindjs madeindjs marked this pull request as ready for review June 29, 2024 19:27
@FabienArcellier
Copy link
Collaborator

FabienArcellier commented Jul 8, 2024

We discuss about generalizing the way we manage this sort of state. We will try to use EmptinessLabel to encapsulate the behavior.

  • make a list of components to change

We would work with this layout

image

@madeindjs
Copy link
Collaborator Author

Hello @FabienArcellier & @ramedina86 ,

I'm trying to implement a general behavior to handle empty state for CoreText & CoreMessage . As @FabienArcellier pointed, we might want to have a general way to do this.

I succeded to implement a EmptinessState component who does the job (5ef475b) but it takes some space in preview mode (as we still render an empty comp).

Screencast.from.2024-07-16.00-24-36.webm

I saw empty state of sidebar / Header / etc.. use a different logic. It uses ChildlessPlaceholder through ComponentProxy.

const getChildlessPlaceholderVNode = (): VNode => {
if (children.value.length > 0) return;
return h(ChildlessPlaceholder, {
componentId: component.value.id,
});
};

So I wonder if we want the same mechanism or if it's a bit overkill for the need and the v-if is enough (ba05e50)

WDTY?

@FabienArcellier
Copy link
Collaborator

Hello @FabienArcellier & @ramedina86 ,

I'm trying to implement a general behavior to handle empty state for CoreText & CoreMessage . As @FabienArcellier pointed, we might want to have a general way to do this.

I succeded to implement a EmptinessState component who does the job (5ef475b) but it takes some space in preview mode (as we still render an empty comp).

Screencast.from.2024-07-16.00-24-36.webm
I saw empty state of sidebar / Header / etc.. use a different logic. It uses ChildlessPlaceholder through ComponentProxy.

const getChildlessPlaceholderVNode = (): VNode => {
if (children.value.length > 0) return;
return h(ChildlessPlaceholder, {
componentId: component.value.id,
});
};

So I wonder if we want the same mechanism or if it's a bit overkill for the need and the v-if is enough (ba05e50)

WDTY?

For the empty state of sidebar / Header / etc, I would stick to the current logic. The v-if looks fine.

@ramedina86
Copy link
Collaborator

@madeindjs Those are two different things

Childless placeholder: Automatically applied to any component that can contain children but doesn't

The other one (what you've called EmptinessState) should be used for when rendering the component would yield undesirable results due to lack of configuration/content.

@madeindjs madeindjs marked this pull request as ready for review July 18, 2024 19:43
@ramedina86 ramedina86 merged commit bd6f95d into writer:dev Jul 23, 2024
16 checks passed
This pull request was closed.
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.

3 participants