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

[Table] Create a playground where you can toggle all features in one place #1154

Merged
merged 27 commits into from
May 26, 2017

Conversation

cmslewis
Copy link
Contributor

@cmslewis cmslewis commented May 26, 2017

Changes proposed in this pull request:

  • Out with the old perf.html spreadsheet example.
  • In with the new Table Playground. (The term "playground" is a little overloaded now; feel free to suggest something else, though note that string doesn't appear in this PR.)

Reviewers should focus on:

  • I've discovered so many Table bugs and/or weirdnesses from this already!
  • I want to merge this quickly and add to it iteratively in future PRs (TODO: text truncation, resizeByTallest..., reorganize the table preview pages).
  • Play with it at [table docs url]/perf.html.
  • Happy to polish styles in future PRs if we need to.

Screenshot

screen shot 2017-05-26 at 1 06 01 pm

image

screen shot 2017-05-26 at 1 06 09 pm

@@ -6,25 +6,131 @@
<link rel="stylesheet" type="text/css" href="dist/perf.css">
<style>
body {
padding: 20px 20px;
position: absolute;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Time to improve the build setup for this preview page methinks (future PR).

margin-bottom: 7px;
}

.tbl-select-label .pt-select {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbl- is my little custom namespace.

);

// TODO: Pull these from @blueprintjs/docs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to do this in this PR. Wasn't able to wrangle it after a few seconds of poking, so I copied+pasted to unblock myself.

@blueprint-bot
Copy link

Better styles

Preview: documentation | table
Coverage: core | datetime

@adidahiya
Copy link
Contributor

  • unchecking "multiple selections" disables drag-to-select-a-region. not sure if this is intentional
  • unchecking "focus cell" disables the feature but keeps around the old focussed cell

overall looks good! can we throw in a focus style manager "only show outline on tabs" somewhere?

@tgreenwatts
Copy link
Contributor

Insert with random text takes a shockingly long time, but other than that looks great! Excited to use this going forward

@tgreenwatts
Copy link
Contributor

The focus cell doesn't move around :(

@cmslewis
Copy link
Contributor Author

cmslewis commented May 26, 2017

@adidahiya:

  1. According to the current implementation, "multiple" = more than one thing. "drag-selection" = more than one cell, hence Table disables it. i don't necessarily agree with this logic though; intuitively, i'd expect allowMultipleSelections={false} to simply disable cmd + select another region.
  2. That's due to an issue with componentWillReceiveProps in Table, not in the example code.

@tgreenwatts:

  1. Fill with random text seems super screwy right now in general. Was already like this though.
  2. Was aware of the broken focus-cell hotkeys. Can fix later.

@blueprint-bot
Copy link

Focus style control

Preview: documentation | table
Coverage: core | datetime

@cmslewis
Copy link
Contributor Author

@adidahiya I added a focus-style control.

image

@blueprint-bot
Copy link

Sync focus style on componentDidMount too

Preview: documentation | table
Coverage: core | datetime

@cmslewis
Copy link
Contributor Author

Merging because this only affects internal Table preview, and I have other stuff I want to do with it today.

@cmslewis cmslewis merged commit 407ba40 into master May 26, 2017
@cmslewis cmslewis deleted the cl/table-example branch May 26, 2017 23:35
@giladgray giladgray mentioned this pull request Jun 8, 2017
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.

4 participants