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: improve text prompts of empty group components #982

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

Skaiir
Copy link
Contributor

@Skaiir Skaiir commented Jan 11, 2024

Closes #965

@volodymyr-melnykc I made the hiding threshold 100pixels instead because it still seems to look okay, let me know how it seems to you.

@vsgoulart Opted for a mutationobserver for simplicity, since this is purely a visual thing

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Jan 11, 2024
@github-actions github-actions bot temporarily deployed to demo-965-dynlist-empty-state-f January 11, 2024 12:53 Destroyed
@github-actions github-actions bot temporarily deployed to demo-965-dynlist-empty-state-f January 11, 2024 12:53 Destroyed
Copy link

@volodymyr-melnykc volodymyr-melnykc left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@vsgoulart vsgoulart left a comment

Choose a reason for hiding this comment

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

@Skaiir Even though this component won't ResizeObserver + refs are no that great for performance.

Let's try to use container queries instead which allows use to avoid shipping more JS


return (
<div class="fjs-empty-component" ref={ emptyComponentRef }>
<span style={ { display: isHidden ? 'none' : 'inline' } }>Drag and drop components here.</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use classes instead of inlining it

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Jan 15, 2024
@github-actions github-actions bot temporarily deployed to demo-965-dynlist-empty-state-f January 15, 2024 21:00 Destroyed
@Skaiir
Copy link
Contributor Author

Skaiir commented Jan 16, 2024

Oooh snazzy new feature I (and GPT) wasn't aware of.

@Skaiir Skaiir force-pushed the 965-dynlist-empty-state-fix branch from df63da5 to 404a750 Compare January 16, 2024 07:55
@github-actions github-actions bot temporarily deployed to demo-965-dynlist-empty-state-f January 16, 2024 07:55 Destroyed
@Skaiir Skaiir requested a review from vsgoulart January 16, 2024 07:59
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jan 16, 2024
@github-actions github-actions bot temporarily deployed to demo-965-dynlist-empty-state-f January 16, 2024 07:59 Destroyed
@Skaiir Skaiir merged commit d8dd39c into master Jan 16, 2024
13 of 15 checks passed
@Skaiir Skaiir deleted the 965-dynlist-empty-state-fix branch January 16, 2024 12:02
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants