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

[4] initial concept of table options #32974

Closed
wants to merge 10 commits into from
Closed

[4] initial concept of table options #32974

wants to merge 10 commits into from

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Apr 2, 2021

Summary of Changes

THIS. IS. ALMOST. INCOMPLETE.

This is my worse work to date, Im totally at the limit of JS here.. but "it works on my machine", it "looks ugly and needs css"

Happy to merge any PRs made to the branch - this needs some help to get it over the line!

A huge thanks to @dgrammatiko so far. He gave me clean code and I butchered it.

Testing Instructions

I have only applied the change to the Users, Articles and Categories pages.

Actual result BEFORE applying this Pull Request

No way to hide columns

Expected result AFTER applying this Pull Request

A new Table options list at the foot of a table, that allows you to deselect columns to HIDE and to select columns to SHOW

Your preferences are STORED in your browser localstorage for that site. When you reload the page or return to the same page, your preferences are reapplied.

LIMITATIONS & NOTES:

The first 2 columns can never be hidden, these are normally the checkbox and the dragger (or the checkbox and the username on the users list)
Occasionally Ive seen a CSS issue on the new checkbox list layout, but Ive applied no css at all so far.
ONLY tested in Safari and Firefox but SHOULD work in all browsers
ONLY shows up in 1024px wide screens and above by design.
The bottom of the table was chosen for the placement of the Table Options as the page has been "designed" previously and adding them at the top just gets in the way.
Probably have to target 4.1-dev as this is a new feature.

Future improvement ideas already suggested:

  • Store the preferences against the user object in the db for browser/computer independentcy
  • Initially hide most things to "clean up" the user interface and allow users to add more clutter
  • Overcome the bootstrap limitation of classes for screens less tan 1024px wide and remove all responsive classes that are currently hiding things, and allow all columns on mobile devices to be toggled with table options.

Preview:

Screen Recording 2021-04-02 at 09 35 53 pm

Documentation Changes Required

Yes.

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Apr 2, 2021
@PhilETaylor PhilETaylor marked this pull request as draft April 2, 2021 20:38
@brianteeman
Copy link
Contributor

Nice work. For me the main problem with this approach is that its per computer, per browser, per site,
But maybe thats just me

@brianteeman
Copy link
Contributor

ONLY shows up in 1024px wide screens and above by design.
Why add this limitation?

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Contributor

For me the main problem with this approach is that its per computer, per browser, per site,

We could add a PHP ajax endpoint to store the state on the user's account options, that's easily doable

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

The first 2 columns can never be hidden,

If you hide the cell that contains the scope then you break the tables accessibility. This is usually (always?) the cell that is the link.
I guess it would be daft for a user to hide that but we all know users can be daft so perhaps prevent this column from being hidden as well.

@brianteeman
Copy link
Contributor

ONLY shows up in 1024px wide screens and above by design.
Why add this limitation?

Because when you view your site on a mobile, the responsive design of the admin template kicks in and most columns are already hidden. the 1024 was the number @dgrammatiko pulled out of the hat.

But what if I wanted to decide the columns for myself. That wasn't possible before but it would be now

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Contributor

Why add this limitation?

The limitation comes from Bootstrap's utilities and the forced !important everywhere. Removing this means we need an array with all the possible classes that need to be removed... It needs a bit of thinking here

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Contributor

The first 2 columns can never be hidden

@PhilETaylor I would keep also the ID always visible (eg arr.length - 1) but that's just personal opinion here

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

Not always easy to know WHICH link in WHICH column... as there are lots of links in the row... links to toggle state, featured etc...

actually it can be determined programmatically because it is a th not a td and has a scope="row"

@brianteeman
Copy link
Contributor

@dgrammatiko @PhilETaylor ok I understand the 1024 part now

@PhilETaylor

This comment was marked as abuse.

Phil E. Taylor added 2 commits April 3, 2021 11:48
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Phil E. Taylor added 2 commits April 3, 2021 12:06
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
cs
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@dgrammatiko
Copy link
Contributor

@PhilETaylor can you diff your script with https://gist.github.com/dgrammatiko/4d8121c2cda4096224d4de283a1a433a you need a couple of conditonals for sanity checks

@PhilETaylor

This comment was marked as abuse.

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Phil E. Taylor added 3 commits April 3, 2021 20:41
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@PhilETaylor

This comment was marked as abuse.

@ceford
Copy link
Contributor

ceford commented Apr 5, 2021

I must be doing something wring. I applied the patch with Patchtester, ran npm ci, but did not see the dropdown list of columns to select for hiding. I did try the demo by @dgrammatiko.

Quite separately, I put the table in a div styled overflow-x: scroll; max-width: 1000px; right now I don't know how to get the width of the parent div. I quite like the left-right scroll that leaves the left menu, toolbar and search bar alone while the table scrolls. Please try it. It could be combined with any other column manipulation.


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

@TLWebdesign
Copy link

I must be doing something wring. I applied the patch with Patchtester, ran npm ci, but did not see the dropdown list of columns to select for hiding. I did try the demo by @dgrammatiko.

I had the same issue. The Patchtester doesn't do the "build" of the JS file of this feature. to Still try it out you could copy the js file contents form the PR to the folder "/media/system/js" and name the file tableoptions.js

At the moment it gave me a JS error on line 124 so i commented both 124 and 125 out so i could see it in action. it looks very promising and i think a very much needed feature to have a "workable" backend.

@ceford
Copy link
Contributor

ceford commented Apr 6, 2021

Here is an experiment to try:

Edit your locally installed administrator/components/com_content/articles/tmpl/default.php
Add these two lines to wrap the table:

					<div class="myscroller">
					... table here
					</div>

And then this after line 95:

$wa->addInlineStyle("
.myscroller {
	max-width: calc(100vw - 20rem);
	overflow-y: scroll;
}
.myscroller.wider {
	max-width: calc(100vw - 5rem);
}
");

$wa->addInlineScript("
	const vw = Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0);
	const test = document.getElementsByClassName('myscroller');

	var ro = new ResizeObserver(entries => {
		for (let entry of entries) {
			const cr = entry.contentRect;
			if (cr.width < 100) {
				test[0].classList.add('wider'); // = vw - cr.width + \"px\";
			} else {
				test[0].classList.remove('wider'); // = vw - cr.width + \"px\";
			}
		}
	});

	ro.observe(document.getElementById('sidebar-wrapper'));
", [], ['type' => 'module']);

Click on the table to scroll left-right. Make the screen narrower - try simulating an iPad in landscape and portrait mode. And toggle the side menu.

@PhilETaylor

This comment was marked as abuse.

@ceford
Copy link
Contributor

ceford commented Apr 6, 2021

@ceford this PR relates to a Javascript feature to hide/show columns. If you would like to propose a competing/complimenting solution with horizontal scrolling of tables then please make a new PR.

Sorry, getting this issue mixed up with #31915. Just thinking of the general problem with wide tables.

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor PhilETaylor closed this May 2, 2021
@dgrammatiko
Copy link
Contributor

@PhilETaylor may I ask why you've killed this one?

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants