-
Notifications
You must be signed in to change notification settings - Fork 842
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
[EuiBasicTable][EuiInMemoryTable][EuiDataGrid] Revert page size number | 'all'
type to just number
#5699
Conversation
759213a
to
345bc61
Compare
Sorry for force push noise, still going through the revert diff and cleaning up various changes we did want in main. Will comment again when done |
abc625d
to
85f488f
Compare
85f488f
to
6a0f56a
Compare
0 is falsey and thus is failing a bunch of checks/fallthroughs, switching to `??` and `!=` null generally solves the problem
@thompsongl Alrighty, apologies for the noise, this should be all working and the diff looks more sane to me now 🙈 cc @cchaos on this as well, for once the staging link goes up - please feel free to ping if you think the examples are too confusing, particularly the EuiTablePagination one. I actually think the |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5699/ |
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, Constance!
I confirmed that the new eui.d.ts
will resolve the type errors in Kibana.
- Missed this just as I was hitting merge on #5699, does not affect any source code
Preview documentation changes for this PR: https://eui.elastic.co/pr_5699/ |
Summary
closes #5697
Unfortunately the
| 'all'
typing (see #5547, particularly the first 4 commits I added) is simply causing too many downstream Typescript shenanigans in Kibana, so in this case typescript should trump API clarity for expedience 😢0
representing all instead of-1
, does-1
feel better?git revert
s caused a decent amount of noise and I slightly regret not just working off of mainQA staging
Checklist