plotly / dash-table

OBSOLETE: now part of https://github.com/plotly/dash
https://dash.plotly.com
MIT License
420 stars 74 forks source link

Custom "Toggle Columns" button value #869

Open remiOdite opened 3 years ago

remiOdite commented 3 years ago

Hi !

It could be interesting to allow to modify the "Toggle Columns" button value. For instance to change the language, or to set a "+" instead of the text.

Do you think it could be possible to add this feature ? What do you think ?

Thanks !

alexcjohnson commented 3 years ago

Related: #560 I'm thinking what we need is a single prop to collect all of these changes - because there will undoubtedly be more of them and we don't want to keep adding more props. What about ui_text: {"Toggle Columns": "+", "Export": "Save"}?

remiOdite commented 3 years ago

That's fit perfectly for me. This is a good idea to bundle all of these features into a single prop such this one.

AnnMarieW commented 3 years ago

I think this is a great solution. How about a similar prop for ui_className?

Perhaps this could be included with this PR: https://github.com/plotly/dash-table/pull/799

alexcjohnson commented 3 years ago

@AnnMarieW interesting idea about bundling className props the same way. Naming is a little trickier there since it's elements, which may or may not have any text in them, getting these classes applied. There may even be two nested elements you want to apply classes to, so the names may have to be things like export_div and export_button. But I think it still makes sense to combine all the classes into a single prop as this is the kind of thing it's unlikely you're going to want to change pieces of after creating the table. You may even want to share the whole collection of classes between multiple tables, which should be a good deal easier if they're grouped in a single prop.

AnnMarieW commented 3 years ago

I was thinking it might work the same way as in PR #799, but instead of creating the two new props export_text and export_className, these could be included in the new dict format, for example:

ui_text : {"export_text": "save"}
ui_className: {"export_className": "m-2"}

or simply: ui={"export_text": "save", "export_className": "m-2"}

It looks like PR#799 is close to being approved. But if it was updated to include this solution, then the structure would be in place to add other things -- such as changing the text and className of the Toggle Columns button, 'filter data...' and nested elements such as the export div, etc. - all without adding more new prop names.

Is there a problem with updating after the table is created? There was a use case on the forum recently: Hiding the export button if the datatable is empty.

alexcjohnson commented 3 years ago

Part of the idea with ui_text is that it could be integrated later on into a dash-wide localization API. The way locales are handled in plotly.js (which, therefore, we would use as a model here absent a good reason to differ) is a dictionary of {"original text": "translated text"}. There are other use cases here, for example if you have two tables on the page and you want each to get a different label on its export button to disambiguate the data being saved, but that seems like it would work well alongside a global translation dictionary if we simply applied the instance-specific text first, then check the global dictionary only if that key is not found in the instance prop.

Also using the original text as the keys avoids the need to choose and document each key separately.

None of that applies to the classNames, but I think we should use this pattern for the text.

Is there a problem with updating after the table is created? There was a use-case on the forum recently: Hiding the export button if the datatable was empty.

This gets annoying mainly if you have multiple independent reasons to alter different parts of a prop - then you need to make one callback that combines all this logic and merges the results - or a multi-callback solution with stores or something.

So perhaps this use case indicates classnames are best left as separate props. Hopefully there won't be too many of these - mostly the table pieces already have classes assigned to them that you can apply the desired CSS to?