Skip to content

[RFC] Responsive Tables#13835

Closed
ciar4n wants to merge 3 commits intojoomla:stagingfrom
ciar4n:com-content-table
Closed

[RFC] Responsive Tables#13835
ciar4n wants to merge 3 commits intojoomla:stagingfrom
ciar4n:com-content-table

Conversation

@ciar4n
Copy link
Contributor

@ciar4n ciar4n commented Jan 31, 2017

Pull Request for Issue # .

Summary of Changes

Creates a responsive tables class allowing more data to be displayed on smaller screens. Class applied to article list.

My intent here is to give an alternative table display on smaller screens where you are not limited to the amount of data that is displayed.

Testing Instructions

Navigate to Content -> Article and resize browser window.

Before patch

com-content-table1

After patch

com-content-table2

Documentation Changes Required

None

@ghost
Copy link

ghost commented Jan 31, 2017

I have tested this item ✅ successfully on aa771fc


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13835.

@Bakual
Copy link
Contributor

Bakual commented Jan 31, 2017

Associations don't have a title when shrunk. It's also missing the clickable table headers to sort the fields.

Personally, I don't like the new small view, I prefer the current phone view.

@ciar4n ciar4n changed the title [com_content] Responsive Tables [RFC] Responsive Tables Jan 31, 2017
@ciar4n
Copy link
Contributor Author

ciar4n commented Jan 31, 2017

Really my intent here is to give an alternative table display on smaller screens where you are not limited to the amount of data that is displayed by converting the table from a column view to a row view. The current table, although it looks neater, is very limited on how much data can be displayed. Maybe com_contents is a bad example to demonstrate however the idea may be useful elsewhere.

I used a similar approach on #13769 where it was suggested that the same technique could be used on other tables.

@Bakual
Copy link
Contributor

Bakual commented Jan 31, 2017

For the repeatable I think this makes sense, because hiding elements doesn't make sense there.

However in regular manager tables, hiding columns is perfectly fine, and those tables already are responsive. No need to fix something that isn't broken imho.

@laoneo
Copy link
Member

laoneo commented Feb 1, 2017

I really like to have the possibility to make tables correctly responsive and not just by hiding elements. Would be nice to have that possibility also on the front end for templates available as less file to include.

@C-Lodder
Copy link
Member

C-Lodder commented Feb 1, 2017

I'd suggest removing all the columns apart from the title and checkbox columns. For mobile, this would be enough

@tuts-22
Copy link

tuts-22 commented Feb 8, 2017

I have tested this item ✅ successfully on e5ff521

I have tested the request and it works fine!!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13835.

@jeckodevelopment
Copy link
Member

@ciar4n please look at the conflicts

@JoshJourney
Copy link

I'm not in favor of this PR due to the display feeling more cluttered. I attached a screen shot showing with the patch and the other without. Both are responsive, this patch shows more meta data per row which ends up making the article view feel not as clean. Normally I like more meta data, but in this case it doesn't seem to add much value on small display. I suppose maybe the author, but not the permission column.

screen shot 2017-02-11 at 20 29 22


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13835.

@ciar4n
Copy link
Contributor Author

ciar4n commented Feb 11, 2017

Conflicts fixed.

I tend to agree that for the article list and for want of as simple a view as possible on mobile devices, this PR might be a little to much. The question is, is there any table view that this layout would be needed? Are we missing any important data on any of the tables on mobile devices that should be displayed?

If so then this PR allows a workable alternative. If not then there is no need for it.

@C-Lodder
Copy link
Member

I normally am in favour of all your PR's Ciaran, but on this one I think it over complicates the visible data

@ciar4n
Copy link
Contributor Author

ciar4n commented Feb 12, 2017

I'm gonna close this. If a need for it arises it can be re-opened.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants