Skip to content

overflow scroll is auto#516

Merged
pzuraq merged 1 commit intomasterfrom
andy/et-auto-scroll
Jun 11, 2018
Merged

overflow scroll is auto#516
pzuraq merged 1 commit intomasterfrom
andy/et-auto-scroll

Conversation

@addepar-andy
Copy link
Copy Markdown
Contributor

reviewers: @Addepar/ice

@pzuraq
Copy link
Copy Markdown
Contributor

pzuraq commented Apr 24, 2018

overflow: scroll is currently how we get fixed columns, headers, and footers. Turning this off will likely break things, as the sticky elements will instead default to whatever the parent scroll container is. Why are you trying to change it?

@addepar-andy
Copy link
Copy Markdown
Contributor Author

because this creates scroll bars when the table when the table doesn't need them. a table that doesn't need to scroll side to side (because you can already see all the columns) will still have the scrollbar, and you'll get ~10px of superfluous scrolling.

@pzuraq
Copy link
Copy Markdown
Contributor

pzuraq commented Apr 24, 2018

That sounds like a bug with antiscroll and the way it inserts scrollbars in our app, native scroll bars shouldn't be visible if the content doesn't overflow as you're describing.

I think it would be better to manually override the overflow style in your table for the time being and keep these styles here, otherwise the default behavior will be broken for users unless they know to make .ember-table a scroll container.

@pzuraq
Copy link
Copy Markdown
Contributor

pzuraq commented Apr 24, 2018

Actually it turns out we aren't using antiscroll app-wide, but we are using custom webkit scrollbar styling. I think we should try to add that to this repo and deal with it here first, come up with a strategy for it

@pzuraq
Copy link
Copy Markdown
Contributor

pzuraq commented May 16, 2018

I think this should be fixed on master, if not let me know and we'll make fixing a priority.

@pzuraq pzuraq merged commit c6da508 into master Jun 11, 2018
@pzuraq pzuraq deleted the andy/et-auto-scroll branch June 11, 2018 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants