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

fix: add possibility to include scrollable table + example in docs #869

Merged

Conversation

JKMarkowski
Copy link
Contributor

…n docs

Please provide a link to the associated issue.

#704

Please provide a brief summary of this pull request.

I added directive which makes scrolling table horizontally possible.

If this is a new feature, have you updated the documentation?

Yes, the documentation is updated and contains one of usage example.

@netlify
Copy link

netlify bot commented Jun 6, 2019

Deploy preview for fundamental-ngx ready!

Built with commit 998f8ac

https://deploy-preview-869--fundamental-ngx.netlify.com

@MattL75 MattL75 changed the title Enhancement - add possibility to include scrollable table + example i… fix: add possibility to include scrollable table + example in docs Jun 6, 2019
templateUrl: './table-docs.component.html',
styles: [`
::ng-deep app-table .fd-tile {
display: block;
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm kind of annoying that this is needed in the first place, but I get it and it's fine to keep it. Maybe something we can consider improving later

@mikerodonnell89
Copy link
Member

This works really good, but I am wondering if we want to test for div width and apply the fixed class to the parent based on that rather than have it as a directive? Could use the scrollbar width changes for frame of reference but this might be something we should check with the fundamental design team

@JKMarkowski
Copy link
Contributor Author

Actually in fiori fundamental library there is no such thing allowing to scroll horizontally. --fixed class is more useful on adding fixed cell, so it's not needed to be used in this case.

@MattL75
Copy link
Contributor

MattL75 commented Jun 7, 2019

Yeah I'm good to merge this - maybe we could add the fixed cell at some point, or even make it toggle-able in the example. I'll wait for word from @mikerodonnell89 though.

@MattL75 MattL75 merged commit a5e1577 into SAP:master Jun 7, 2019
@droshev droshev added this to the sprint 13 - Hot Pie milestone Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
board
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants