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

UI Table - Add "Button" column type #1171

Merged
merged 7 commits into from
Aug 9, 2024

Conversation

bartbutenaers
Copy link
Contributor

@bartbutenaers bartbutenaers commented Aug 7, 2024

Description

I would like to have buttons in a ui-table:

image

This is what this PR currently produces:

image

Marking this as a draft, because I am stuck with a number of things:

  1. I have added the button support in the separate UITableCell.vue (because all other types are supported there), but not sure if that is correct because the checkbox type is added in the UITable.vue file. Which I find rather confusing. Would expect the checkbox also to be in the UITableCell.vue file.

  2. When I inject an input message, my values for the button column are not being showed as button labels. I assume that is because my button is in the separate vue file (perhaps related to point 1).

  3. When I click a button, the onButtonClick method is being called. But the emit in that message does not result in an output message being send. I assume that is because my button is in the separate vue file (perhaps related to point 1).

  4. I use the same color as the other types, but it looks rather weird (as you can see in my screenshot). Not sure why the other types look nice with the same classes...

It would be nice if anybody could give me some tips about what I am doing wrong!

BTW I have added a topic to each output message, in order to allow flows to know what is going on in the ui.

I have not updated the documentation yet.

Related Issue(s)

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@bartbutenaers
Copy link
Contributor Author

Note that this PR is related to my other draft PR. Because when you have multiple button columns, it is usefull to know which button (column) has been clicked. So the msg.column is being filled with the column key again.

@joepavitt
Copy link
Collaborator

Great idea - given you've done the ground work - I'd be happy to jump in and offer some hands-on assistance?

@joepavitt
Copy link
Collaborator

I have added the button support in the separate UITableCell.vue (because all other types are supported there), but not sure if that is correct because the checkbox type is added in the UITable.vue file. Which I find rather confusing. Would expect the checkbox also to be in the UITableCell.vue file.

I think the checkbox you're seeing is the multi-row selection, which is a built in feature of the table, rather than it being bound to the cell's "type".

When I inject an input message, my values for the button column are not being showed as button labels. I assume that is because my button is in the separate vue file (perhaps related to point 1).

To render the label on the button, you should include the relevant text inside the tags, e.g. <v-btn>label goes here</v-btn>

When I click a button, the onButtonClick method is being called. But the emit in that message does not result in an output message being send. I assume that is because my button is in the separate vue file (perhaps related to point 1).

Correct, the v-btn is call insinde the scope of the table cell, but the table itself remains unaware. The best practice here would be to call this.$emit("action-click") within the table cell, then you can define an @action-click="doSomething" on the instance of the table cell in your table.

I use the same color as the other types, but it looks rather weird (as you can see in my screenshot). Not sure why the other types look nice with the same classes...

Will need to have a hand-on play with this one

@bartbutenaers
Copy link
Contributor Author

@joepavitt,
Thanks for teaching me!!

Now the injected labels are displayed on my buttons:

image

And my output message is being send as expected:

image

For my use case this is working fine now, because I can now delete easily the displayed video recordings from my doorbell.

@bartbutenaers
Copy link
Contributor Author

BTW from the topic and column message properties, you can easily determine which button has been clicked. In case you have multiple button-type columns in a table.

@joepavitt
Copy link
Collaborator

BTW from the topic and column message properties, you can easily determine which button has been clicked. In case you have multiple button-type columns in a table.

I was just assessing that as you sent that message - my only other thought was having the key you bind to that column as part of the button click event, but that's essentially what you're doing anyway?

@bartbutenaers
Copy link
Contributor Author

Not sure if I understand it correctly.
The msg.column contains the key of the button column:

image

And this msg.column property is only available in the output message when a button is being clicked.

@joepavitt joepavitt self-requested a review August 9, 2024 07:47
@joepavitt
Copy link
Collaborator

Having a play - I assume you've added a field/property with the "Verwijder" string in the object in order to label the button that way?

Trying to think of a situation where users may just want a static string, or even icon - we could evolve this as a follow on so that "Key" becomes "Value" and then the input is a TypedInput. For the "Button" type, we offer "key", "string", "icon"? For all others, just "key"?

@bartbutenaers
Copy link
Contributor Author

Yes in my case all buttons have the same static label "Verwijder", which I apply via the key of the button column:

{
   datum: some_dynamically_calculated_date,
   tijd: some_dynamically_calculated_time,
    actie: 'Verwijder' // Static button label
}

A typedinput might indeed be a next step. Or perhaps allow html text?

@joepavitt joepavitt changed the title DRAFT - buttons in ui-table UI Table - Add "Button" column type Aug 9, 2024
@joepavitt
Copy link
Collaborator

As a first iteration for the button - I'm happy with this. I can add some docs for you, and get this merged. I'll then open a follow up issue to consider typed inputs

@joepavitt
Copy link
Collaborator

joepavitt commented Aug 9, 2024

Rather than msg.topic, how about msg.action? Feels more appropriate naming? Then we should have:

  • row_click
  • multi_select
  • button_click

As the three interactions? rows_click is too similar to row_click and could easily be confused.

@joepavitt
Copy link
Collaborator

Have made the above changes, including adding some documentation, also set variant="flat" to remove the shadow on the rendered buttons, which aligns them to the rest of the buttons used in Dashboard 2.0

This is great @bartbutenaers - thanks for the work on this!

Copy link
Collaborator

@joepavitt joepavitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing tests run fine locally, so I'll put this down to GH automation oddities - approval from me

@joepavitt joepavitt merged commit 7849643 into FlowFuse:main Aug 9, 2024
1 of 2 checks passed
@bartbutenaers
Copy link
Contributor Author

@joepavitt,
Agree with your sugestions.
Thanks again for the tweeking and the documantation!!
Will create an issue for the typedinput.

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.

2 participants