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

b:dataTable overflow #794

Closed
chongma opened this issue May 25, 2017 · 15 comments
Closed

b:dataTable overflow #794

chongma opened this issue May 25, 2017 · 15 comments
Assignees
Milestone

Comments

@chongma
Copy link
Collaborator

chongma commented May 25, 2017

b:dataTable is the only component which seems to affect the mobile layout width. if when it is rendered it was wrapped in a div which had style

overflow-x: auto;
white-space: nowrap;
overflow-y: hidden;

then it would always scroll horizontally within its container?

chongma pushed a commit to chongma/BootsFaces-OSP that referenced this issue May 28, 2017
@TheCoder4eu TheCoder4eu added this to the v1.1.2 milestone May 28, 2017
@TheCoder4eu
Copy link
Owner

@chongma is this issue solved after the commit?

@chongma
Copy link
Collaborator Author

chongma commented Jun 4, 2017

@TheCoder4eu it works for me but i wasn't sure if it was good enough to make it into the release without review

@chongma
Copy link
Collaborator Author

chongma commented Jun 8, 2017

@TheCoder4eu whoops. i have been deleting all my PRs. i kept resetting my branch to upstream and it deleted all my changes. there are quite a few i have deleted.

@TheCoder4eu
Copy link
Owner

@stephanrauh @chongma , I found the original amened commit andcommitted it again.
Could you provide a feedback on it, so we can close this Issue?

@chongma
Copy link
Collaborator Author

chongma commented Jul 11, 2017

hi, i am a bit behind so i will try to check it later. it should be in the main branch now?

@stephanrauh
Copy link
Collaborator

@chongma I don't see the benefit of your change yet. Would you mind to explain it again?

Be that as it may, it seems to be OK. I only had to do a minor change because the divs piled up in our AJAX demo. IMHO, the change is good to go.

@chongma
Copy link
Collaborator Author

chongma commented Jul 12, 2017

On a mobile device the data table seems to be the only control which pushes the page width beyond the screen limits. This can be corrected by manually wrapping it in a div and my suggestion was to make it the default. Not sure if it might mess other things up though...

@stephanrauh
Copy link
Collaborator

stephanrauh commented Jul 12, 2017

OK, I've got it. I'll ponder about the implications. Maybe we should make it configurable? If so, how to call the flag?

@TheCoder4eu
Copy link
Owner

@stephanrauh ,@chongma we could introduce a flag and call it "wrapped" ?
@stephanrauh your change refers to this I suppose:

So updating the table by its id means it's wrapped into another div each time it's updated by an AJAX request.
If there's not enough screen estate to display the table, the last couple of columns are hidden, as you can see in this example:

@chongma
Copy link
Collaborator Author

chongma commented Jul 13, 2017

Hmmmm then shouldn't that be the default setting?

@stephanrauh
Copy link
Collaborator

stephanrauh commented Jul 14, 2017

No, it shouldn't. I've added a new flag "scroll-horizontally" activating your, @chongma 's, wrapper. A short time I thought about abandoning it completely because there's already a similar flag called scroll-x. The only problem being that scroll-x has a couple of nasty side effects.

stephanrauh added a commit that referenced this issue Jul 14, 2017
stephanrauh added a commit to TheCoder4eu/BootsFacesWeb that referenced this issue Jul 14, 2017
@stephanrauh
Copy link
Collaborator

I've just uploaded the new snapshot version of BootsFaces to Maven Central. See #369 on how to get it.

@chongma Please double-check if I've implemented `scroll-horizontally´ correctly. We'd like to publish the new version of BootsFaces 1.1.2 this weekend. If you don't manage to test the feature, never mind that, but if you do, that'd be great!

A luego
Stephan

@stephanrauh
Copy link
Collaborator

@chongma I'd like to replace your wrapper by the standard wrapper provided by Bootstrap: https://v4-alpha.getbootstrap.com/content/tables/#responsive-tables.

However, the standard wrapper doesn't include white-space:nowrap. Does your application work without it, too?

@chongma
Copy link
Collaborator Author

chongma commented Aug 23, 2017

good find. i am sure the tables should be wrapped especially on small screens. how can i test the feature? should we wrap the data tables in the showcase and then i can test on a small screen and report back?

@stephanrauh stephanrauh removed their assignment Sep 24, 2017
@TheCoder4eu
Copy link
Owner

@chongma @stephanrauh this is a issue reopened on a Closed Milestone.
I'm closing it, but please open a new Issue on 1.2.0 Milestone if there still is some work to do on it.

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

No branches or pull requests

3 participants