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

Hide toggle button when the row is not expandable #1025

Merged
merged 12 commits into from
Nov 21, 2019

Conversation

imballinst
Copy link
Contributor

@imballinst imballinst commented Oct 22, 2019

Fixes #1023. Adds a class MUIDataTableSelectCell-expandDisabled to the expand button, when isRowExpandable(dataIndex) returns false.

This allows us to hide the IconButton [shown by default] -- and therefore, won't break existing behavior.

Medias

Expandable toggle button -- does not have class

image

Non-expandable toggle button - has class expandDisabled

image

GIF of the Example

In this GIF, I will show case 2 examples -- the new example for the hidden button one, and the existing example (just to show the current features are not breaking).

ezgif com-video-to-gif(5)

@coveralls
Copy link

coveralls commented Oct 22, 2019

Coverage Status

Coverage decreased (-0.006%) to 77.352% when pulling aeafe10 on Imballinst:update-isrowsexpandable into 86deadf on gregnb:master.

@gabrielliwerant
Copy link
Collaborator

I don't want to force this behavior by default, and it's not how the parallel feature of row selection works, see my comment on your issue for another way to move forward here.

Signed-off-by: Try Ajitiono <[email protected]>
Signed-off-by: Try Ajitiono <[email protected]>
@imballinst
Copy link
Contributor Author

PTAL @gabrielliwerant and let me know if you have further requests! Thanks.

@gabrielliwerant gabrielliwerant added needs review Useful to mark PRs for what's up next to review and removed needs work labels Nov 16, 2019
Copy link
Collaborator

@gabrielliwerant gabrielliwerant left a comment

Choose a reason for hiding this comment

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

Hey, thanks for continuing to work on this! I've got a few more comments for you, but I think we're getting close.

examples/expandable-rows/index.js Outdated Show resolved Hide resolved
examples/expandable-rows-hidden-expand/index.js Outdated Show resolved Hide resolved
src/components/TableBody.js Outdated Show resolved Hide resolved
@gabrielliwerant gabrielliwerant added enhancement needs work and removed needs review Useful to mark PRs for what's up next to review labels Nov 16, 2019
Signed-off-by: Try Ajitiono <[email protected]>
Signed-off-by: Try Ajitiono <[email protected]>
Signed-off-by: Try Ajitiono <[email protected]>
@@ -240,6 +240,11 @@ class TableBody extends React.Component {
fixedHeader={options.fixedHeader}
checked={this.isRowSelected(dataIndex)}
expandableOn={options.expandableRows}
// When rows are expandable and selectable, but this row isn't expandable, set this to enabled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is probably out of date now that we no longer check for selectable, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iuhh correct, I will delete the selectable part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated it! let me know what you think.

Signed-off-by: Try Ajitiono <[email protected]>
Copy link
Collaborator

@gabrielliwerant gabrielliwerant 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! Thanks for working on this, I think this is a great feature to have.

@gabrielliwerant gabrielliwerant merged commit 03f26cc into gregnb:master Nov 21, 2019
pgoforth added a commit to pgoforth/mui-datatables that referenced this pull request Nov 25, 2019
…/activeIcon-for-search-not-reset-on-toggle

* 'master' of github.com:gregnb/mui-datatables:
  2.13.0
  Prettify files
  Feature/issue 1036 fixed header (gregnb#1071)
  Update webpack dev server and related dependencies (gregnb#1069)
  Feature/issue 871 (setCellHeaderProps) and Feature/issue 714 (setTableProps) (gregnb#889)
  Feature/issue 937 array and custom update filter support (gregnb#1067)
  Bugfix/issue 1052 csv download issues (gregnb#1064)
  Hide toggle button when the row is not expandable (gregnb#1025)
  add test case to verify reset type (gregnb#1006)
  Update .gitignore and adding yarn.lock (gregnb#1004)
  Serverside sorting example (gregnb#986)
  No Pointer style cursor for table head when no hint or sorting (gregnb#910)
  Added disableToolbarSelect option (gregnb#874)
  Add table properties option (gregnb#670)
lalong13 pushed a commit to lalong13/mui-datatables that referenced this pull request Jan 15, 2020
* add more expandableOn condition to prevent toggle button render

Signed-off-by: Try Ajitiono <[email protected]>

* hide toggle button if row is not expandable

Signed-off-by: Try Ajitiono <[email protected]>

* Create unobtrusive change instead

Signed-off-by: Try Ajitiono <[email protected]>

* Add example

Signed-off-by: Try Ajitiono <[email protected]>

* Revert old example and disable prettier while saving

Signed-off-by: Try Ajitiono <[email protected]>

* cleanup

Signed-off-by: Try Ajitiono <[email protected]>

* Remove unnecessary width style

Signed-off-by: Try Ajitiono <[email protected]>

* Remove options selectable rows from hideExpandButton prop

Signed-off-by: Try Ajitiono <[email protected]>

* Only have one example

Signed-off-by: Try Ajitiono <[email protected]>

* Remove example

Signed-off-by: Try Ajitiono <[email protected]>

* Update comment

Signed-off-by: Try Ajitiono <[email protected]>

* update comment again

Signed-off-by: Try Ajitiono <[email protected]>
@imballinst imballinst deleted the update-isrowsexpandable branch October 7, 2020 03:19
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.

Hide toggle button when the row isRowExpandable resolves to false
3 participants