girder / girder_web_components

Reusable Javascript and VueJS components for interacting with a Girder server.
https://gwc.girder.org
Apache License 2.0
16 stars 9 forks source link

225 suggestion #253

Closed subdavis closed 4 years ago

subdavis commented 4 years ago

@dchiquit I don't typically like to butt in on other people's features like this, but I needed to mess around with code to see if my idea would even work.

Here's a suggestion that I think simplifies things a bit. The one part of your PR that I was opposed to was the use of this inside the ActionKeys handler function, because it isn't clear how the function context gets set and I found it confusing. I know I did it that way initially, and that wasn't great.

Now, the actions computed prop adds a computed url to the action iff a urlfunc exists, and if that is found, the v-list-item href (and optionally, target) are used.

Let me know what you think. We don't have to actually merge this PR, just thought it'd be helpful to see.

dchiquito commented 4 years ago

Closing in favor of https://github.com/girder/girder_web_components/pull/254