-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Dataviews: Improve story #59773
Dataviews: Improve story #59773
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -1,12 +1,12 @@ | |||
.dataviews-wrapper { | |||
width: 100%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This style makes the assumption that the parent div doesn't have a padding which is not always true.
height: 100%; | ||
overflow: auto; | ||
box-sizing: border-box; | ||
scroll-padding-bottom: $grid-unit-80; | ||
} | ||
|
||
.dataviews-filters__view-actions { | ||
box-sizing: border-box; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This style is present in WordPress "reset.css" but is needed for places like storybook or usage outside of WP. This makes the style more portable.
@@ -49,6 +49,20 @@ const fields = [ | |||
maxWidth: 400, | |||
enableHiding: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oandregal I wanted to add a simple "text" field, Like an input where I can type the value or "start with" something ... I was surprised to find out that we don't support that yet and we only support "enumeration".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all the current implementations use the global search instead. Added it to the backlog and asked for design input.
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
maxWidth: 400, | ||
enableHiding: false, | ||
type: 'enumeration', | ||
elements: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the filtering happens manually, so this just adds the filter in UI, but not filters results. For client side handling this should probably be absorbed by the package in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DataViews component can't know if the filtering is happening client side or server side, which means it can't know if the available values are exhaustive. I'm fine with this being defined on the field personally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed the filtering for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
} ); | ||
} | ||
} ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should build a reusable hook for this, the code will become more complex over time when we add more filter types and operators.
Co-authored-by: ntsekouras <ntsekouras@outlook.com>
What?
This PR improves the dataviews story by fixing some style bugs and by adding a filter.
Stories are one of the best ways to develop and document reusable components in separation. We should up our game in terms of DataViews stories.
I think we should treat the stories as a "reference". In other words, we should have a story for each "filter" type for instance, a story for each "layout"...