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.1] User defined Hide Table Columns #36591

Merged
merged 53 commits into from
Feb 1, 2022

Conversation

brianteeman
Copy link
Contributor

Summary of Changes

Decide for yourself which columns to display for any table. The button shows an updating count of how many columns are shown.

The choice of which column to display is stored in local storage so it is persistent for you in your browser for that site.
tables2

This should be added to every table and it can also be added by extension developers to their own table by just adding one line to the existing web assets.

// add the script
/** @var \Joomla\CMS\WebAsset\WebAssetManager $wa */
$wa = $this->document->getWebAssetManager();
$wa->useScript('table.columns');

Testing Instructions

As there is new js you will need to run npm ci

This is a fork and continuation of work by @PhilETaylor @dgrammatiko @Fedik

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.1-dev labels Jan 6, 2022
@brianteeman
Copy link
Contributor Author

Yes thats out of scope

No I'm not moving it to the bottom. It makes no sense to hide it on the bottom of the second or third page scroll

@brianteeman
Copy link
Contributor Author

your face must be really bruised the number of times that you smash your palm into it

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@roland-d roland-d changed the base branch from 4.1-dev to 4.2-dev February 1, 2022 19:47
@Quy Quy removed the PR-4.1-dev label Feb 1, 2022
@roland-d roland-d merged commit d0d4ed7 into joomla:4.2-dev Feb 1, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 1, 2022
@roland-d
Copy link
Contributor

roland-d commented Feb 1, 2022

@brianteeman As Joomla 4.1 is in feature freeze Benjamin asked me to merge it for 4.2. I agree this is a great feature, so have it merged for 4.2.

Thanks everybody for working on this.

@roland-d roland-d added this to the Joomla 4.2.0 milestone Feb 1, 2022
@brianteeman
Copy link
Contributor Author

thanks for merging - shame people will have to wait :(

@brianteeman brianteeman deleted the table_columns branch February 1, 2022 20:50
@Kostelano
Copy link
Contributor

Please add a language tag to the PR

@Fedik Fedik added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester labels Feb 11, 2022
@joeforjoomla
Copy link
Contributor

joeforjoomla commented Feb 15, 2022

This script is too much generic if used for third-party extensions. It searches for all tables in a page throwing a JS error if there is no head section or applying the hide columns control to all found tables even if undesired.
It's a pity it can't be used only for the main or specific table in backend. It would be great an addition to exclude or include only tables that really need it.
image

@Fedik
Copy link
Member

Fedik commented Feb 15, 2022

Well, that was expected. But people wanted like that.
You better open an issue with some "test case" description. If you like.

@brianteeman
Copy link
Contributor Author

a table without a head section is invalid so not going to apologise if it doesnt work

@brianteeman
Copy link
Contributor Author

It would be great an addition to exclude or include only tables that really need it.

I dont understand - you have to add it to your component to use it. its not automatic

@joeforjoomla
Copy link
Contributor

joeforjoomla commented Feb 16, 2022

It would be great an addition to exclude or include only tables that really need it.

I dont understand - you have to add it to your component to use it. its not automatic

@brianteeman of course i know.

The problem now, is that this code should skip unnamed tables, but actually it doesn't do this because if a table has no name it fallbacks to the header title:

const tableName = $table.dataset.name ? $table.dataset.name : document.querySelector('.page-title').textContent.trim().replace(/[^a-z0-9]/gi, '-').toLowerCase(); // Skip unnamed table

    if (!tableName) {
      return;
    }

As a consequence if there are let's say 3 tables in the backend interface of a component but not all 3 tables used for columns to be hidden, then this feature will apply to all of them.
Furthermore, if a table has an empty <head></head> without any <tr>, than you have a the javascript error because of this line of code looking at the 'children' attribute of null:

this.$headers = [].slice.call($table.querySelector('thead tr').children);
Then it should be the case to fix the 'children' code to avoid a JS error, or to introduce an option for a configurable JS selector to include or exclude only certain tables.

I hope to have well explained.

@brianteeman
Copy link
Contributor Author

Two choices

  1. Don't implement the script in your component
  2. Submit a pull request with the required changes

@impressionestudio
Copy link

Very useful option but...

Why the new dropdown list button for the columns is placed in a new line and not in the same row and after the dropdown list button for the list limit?

As, for example, the list limit button affects the table, also does this new button for the columns of the table. So they should placed together.

There is space for them to fit in one row and if the left menu is closed then there is more space. I think we need to stop pushing the table too much down.

Also the other filters have a specific ID or class e.g. "list_limit", "list_fullordering". Why the new button for the columns doesn't have?

@HSorgYves
Copy link

HSorgYves commented Sep 7, 2022

d-xxl-table-cell, d-sm-table-cell and d-xs-table-cell aren't removed by the JavaScript. Can you please adjust the JS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators 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.