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

Showing widget placeholders allowing inline widget creation. #14331

Merged
merged 40 commits into from Jan 12, 2023

Conversation

dennisoelkers
Copy link
Member

@dennisoelkers dennisoelkers commented Dec 30, 2022

Description

Motivation and Context

This PR is adding a feature that adds widget placeholders in empty slots on the widget grid, allowing the user to create widgets in these spots. This removes the need for the user to create a widget and then move it to the desired spot.

/jenkins-pr-deps Graylog2/graylog-plugin-enterprise#4510

How Has This Been Tested?

Screenshots (if appropriate):

inline-widget-creation

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@dennisoelkers dennisoelkers force-pushed the feat/widget-creation-placeholders branch 2 times, most recently from 6c6ca8d to 496f3c6 Compare January 4, 2023 07:38
Copy link
Contributor

@linuspahl linuspahl left a comment

Choose a reason for hiding this comment

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

Works really well!

}), [creators, position, view]);

return (
<Modal title={modalTitle}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need for this title? It looks like now we always display a title when hovering over the modal
image

};

return (
<CreateWidgetButton type="button" onClick={onClick}><HugeIcon><WidgetIcon /></HugeIcon>{title}</CreateWidgetButton>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to display a title for each button like "Create message count widget"

display: flex;
flex-wrap: wrap;
justify-content: space-evenly;
`;
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 add a gap here to ensure there is always a gap between each widget tile.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also like how it looks, if we would center the tiles.
image

In this example I also used background: transparent and border-radius: 4px; for the tiles.

const PlaceholderBox = styled.div(({ theme }) => css`
opacity: 0;
transition: visibility 0s, opacity 0.2s linear;

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 add cursor: pointer to this element, just to make it even more clear that you can click everywhere and not only on the icon.

@dennisoelkers dennisoelkers force-pushed the feat/widget-creation-placeholders branch 2 times, most recently from 967dd22 to 6cdfb17 Compare January 10, 2023 13:47
@dennisoelkers dennisoelkers force-pushed the feat/widget-creation-placeholders branch from 56cff62 to 79a00f0 Compare January 12, 2023 13:08
@dennisoelkers dennisoelkers force-pushed the feat/widget-creation-placeholders branch from 79a00f0 to bde2f13 Compare January 12, 2023 13:17
Copy link
Contributor

@linuspahl linuspahl left a comment

Choose a reason for hiding this comment

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

Works like a charm!

@linuspahl linuspahl merged commit 26720c4 into master Jan 12, 2023
@linuspahl linuspahl deleted the feat/widget-creation-placeholders branch January 12, 2023 15:48
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.

None yet

2 participants