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

fixedHeaderOptions not working when selectableRows='none' #1079

Open
GuyShaanan opened this issue Nov 25, 2019 · 10 comments
Open

fixedHeaderOptions not working when selectableRows='none' #1079

GuyShaanan opened this issue Nov 25, 2019 · 10 comments
Labels

Comments

@GuyShaanan
Copy link
Contributor

GuyShaanan commented Nov 25, 2019

Thanks for the feature, it is really useful.

Expected Behavior

When selectableRows == 'none' the first column (whatever it is) should stay in view when scrolling horizontally with setting

fixedHeaderOptions: {
     xAxis: true
}

Current Behavior

When the first column isn't the row selection checkboxes column- the first column doesn't stay in view when you scroll horizontally.

Steps to Reproduce (for bugs)

  1. Take the fixedHeaderOptions demo.
  2. Scroll horizontally- notice the first column of checkboxes stays in view even when you scroll.
  3. Add selectableRows: 'none' to options.
  4. Now scroll horizontally again. The first column (which is now the "Name" column) doesn't stay in view.

Your Environment

Tech Version
Material-UI 3.9.2
MUI-datatables 2.13.0
React 16.8.4
browser Chrome 78
@gabrielliwerant
Copy link
Collaborator

Hey @GuyShaanan, I'm having difficulty reproducing. Modifying the simple example to include the options (and enough data to generate scroll bars), I see the following:

capture

Can you create a codesandbox to help me reproduce the problem?

@gabrielliwerant gabrielliwerant added the needs verification For issues that can't be reproduced or are otherwise unclear label Nov 25, 2019
@GuyShaanan
Copy link
Contributor Author

Your gif shows it perfectly- the "Name" column which is the first column isn't "fixed" in place. It scrolls when you scroll horizontaly, when it should actually stay in place.

@gabrielliwerant
Copy link
Collaborator

Actually, that behavior is intentional, and as far as I can tell, it has always worked that way. The fixed y-axis behavior means that whatever column you are over is the column header that shows. It will not show a "Name" header once you've scrolled past "Name" and are looking at "Title." I believe that doing otherwise would cause more confusion, as the header would not correspond to the column that shows. If you remove the fixed y-axis behavior, you will not see the header stick to the column, it will just scroll past. Maybe that's the behavior you're looking for?

@GuyShaanan
Copy link
Contributor Author

GuyShaanan commented Nov 26, 2019

I updated the original message to be more readable:
When you open the demo, the first column is the row selection checkboxes.
When you scroll horizontally, that first column stays in view as you scroll (as in you always see all the checkboxes when you scroll). Thats OK.

Now set selectedRows: 'none'. Now the "Name" column is the first column,
Now when you scroll horizontally, that first column doesn't stay in view as you scroll. Thats the problem.

Am I misinterpreting that feature? That stickiness of the column header text is a known feature and I'm not referring to it in this ticket).

Thanks!


Edit:
I now see that the row selection checkboxes column always stays in view regardless this feature.
I thought this feature meant that the first column stays in view when you have xAxis: true and you scroll horizontally, just as the fixedHeader option made the header row (the "y" Axis) to stay in view when you scrolled vertically.

@gabrielliwerant
Copy link
Collaborator

Hopefully this clears it up. The intended behavior of xAxis is as follows:

  • With selectable rows on and xAxis on, the whole column of checkboxes is horizontally sticky, and stays in view as you scroll horizontally
  • With selectable rows off and xAxis on only the header name is horizontally sticky, but as you scroll horizontally, the header name will change to match the column that is visible so that you do not see a header name above a column that would be incorrect
  • With selectable rows on and xAxis off, the checkboxes are not sticky. They do not stay in view as you scroll horizontally
  • With selectable rows off and xAxis off, the header is not horizontally sticky, and will not stay in view as you scroll horizontally

To test the behavior of these, you need enough columns and small enough view to generate horizontal scroll bars. In my testing, everything works here exactly as intended.

@GuyShaanan
Copy link
Contributor Author

Great, thanks, that clears the issue.
But- the behavior it too unpredictable (and hard to infer from the documentation). The combination of these two- seemingly unrelated- options (selectableRows and fixedHeaderOptions) results in too many different UI behaviors.

Also- why would it make sense to only make the checkboxes column sticky, but not the first column when selectableRows == 'none'? (which is what #236 asks)

@gabrielliwerant
Copy link
Collaborator

Agreed, it's not clear from the API that selectableRows would alter the behavior. With "fixed header" the natural expectation would be that it affects only the header, which it does, except in cases where we have selectable rows. I think that distinction exists just because the utility when we have selectable rows is really high. However, I'm open to suggestions for ways to rename/clear up the API around this.

@gabrielliwerant gabrielliwerant added question and removed needs verification For issues that can't be reproduced or are otherwise unclear labels Nov 26, 2019
@patorjk
Copy link
Collaborator

patorjk commented Nov 29, 2019

Just an FYI, this bug is still present in the table: #900

I was about to submit a PR to fix it (I have it fixed in my mui-dt repo), but I noticed the API change. The bug described in that ticket is a bad bug, the table does not work right in "scroll" mode in Safari. Because of how "position: sticky" works in Safari, I don't think you can achieve all of the modes you described above.

I would recommend a partial revert of the API change. Instead of discarding fixedHeader, I'd change it to only govern the stickiness of the header, and then add an option called "fixedLeft" that defaulted to true that governed if the select column was sticky.

Any changes to this really should be tested in Safari on a Mac, since the implementation of "sticky" is different in that browser. Also, if you have an iPhone, you can use Chrome and see the same bug listed in the ticket I mentioned.

@patorjk
Copy link
Collaborator

patorjk commented Nov 29, 2019

I just put in a PR (#1088) to fix #900 and to allow for an optional fixed header and optional fixed select column.

@cognot
Copy link

cognot commented Mar 31, 2020

Hi,

any news on this one? I do have the same problem and this fix would save my day...

Regards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants