Fixes #29233: Add recent activity table in global parameters details - #7352
Conversation
|
PR updated with a new commit |
|
PR rebased |
f2869be to
73a8588
Compare
73a8588 to
d84551d
Compare
…etails Fixes #29233: Add recent activity table in global parameters details
|
PR updated with a new commit |
…eters details Fixes #29233: Add recent activity table in global parameters details
|
PR updated with a new commit |
…l parameters details Fixes #29233: Add recent activity table in global parameters details
|
PR updated with a new commit |
There was a problem hiding this comment.
eventually (not in this PR of course), the entire global properties table to be migrated to Elm, so that usages of datatable can be removed from this page as well.
we could start by also migrating the rest of the content of the dropdown (i.e. the description field) so that it can be part of the same Elm app as the recent activity table, and, eventually, the entire table of global properties could be migrated to Elm.
we cannot migrate the entire global properties table right now with the current Table module of the Elm library, because some key features of the global properties table would be missing :
- each row, when clicked, opens a new pane that contains the description and recent activity table of the given global property. the row can be clicked again to collapse the pane.
- when the details pane is open, there is a small downwards triangle on the bottom of the row :
- the current global properties table allows the user to only display a certain number of entries (i.e. paging)
do you see any other features that must be added to the Table module before the global properties table can be migrated to Elm @clarktsiory ?
| }; | ||
|
|
||
| function fnFormatActivity(id) { | ||
| // FIXME : add some CSS for activity table from elm app |
There was a problem hiding this comment.
indeed, i think the CSS should be modified : i think the table should have a border and empty space all around it in order to separate it from the rest
did you have other ideas @P4uline ?
There was a problem hiding this comment.
it's acceptable that you modify it in another PR, that's a kind of iterative fix/improvement you can do in alpha
| )}.row(this).child(fnFormatDetails(jsid), color + ' parametersDescription details').show(); | ||
| )}.row(this); | ||
|
|
||
| const globalPropertyName = jTr.find('td.name').find('b').html(); |
There was a problem hiding this comment.
it would be nice to fetch the name of the property in a more robust way eventually.
fetching the name of the property in this way would not be necessary had the entire global properties table been contained in a single Elm app, but migrating it to Elm is not really feasible with the current state of the Rudder.Table module as it does not support many of the key features of the properties table.
in the meantime, do you see a cleaner way of fetching the property name @clarktsiory @RaphaelGauthier ?
There was a problem hiding this comment.
I completely agree that it's not ideal to rely on potentially unstable CSS selectors (such as b) to find the name of the global property... The current td.name looks fine to me, but I'd prefer directly find('td.name').text() with further sanitization.
As for a cleaner way to do that, I don't know any other solution than CSS selectors at this level...
|
Global properties are indeed specific, and as you reported need some additional table features:
There are also other tables that would need these in order to be migrated in Elm (for example change logs). But the UI may also need to be revised: it has quite diverged from the node and group properties UI (which are the same Elm app). I'd favor consistency with other "properties UI" to plain migration of an "old UI". So, I don't think we should prioritize having a "generic table", to having "consistent properties UI across webapp". This should be discussed with @RaphaelGauthier |
3 participants
https://issues.rudder.io/issues/29233