jhelvy / splitKbCompare

An interactive tool for comparing layouts of different split mechanical keyboards
https://jhelvy.github.io/splitKbCompare/
MIT License
270 stars 30 forks source link

Datatable filter switch #81

Closed undefined-landmark closed 2 years ago

undefined-landmark commented 2 years ago

Hi,

I tried to implement a switch on the Keyboards pane that applies all active filters from the compare pane. By default this functionality is not active.

After writing it I saw that getFilteredKeyboardNames gets called three times in server.R, it should be a bit more efficient (and cleaner) to treat is as a reactive value.

If you have any ideas for the placement or the type of switch I'm happy to implement. I think it's okay as of now but it could always be nicer.

netlify[bot] commented 2 years ago

✔️ Deploy Preview for splitkbcompare ready!

🔨 Explore the source changes: 1d6591a08f6f96264584e6a35cc690678632ecd6

🔍 Inspect the deploy log: https://app.netlify.com/sites/splitkbcompare/deploys/61d32c6941cdc100074b33f6

😎 Browse the preview: https://deploy-preview-81--splitkbcompare.netlify.app

undefined-landmark commented 2 years ago

Should the deploy preview contain my pull request? It doesn't seem to do so at the moment. The concept does sound interesting.

jhelvy commented 2 years ago

Thanks for these edits, and good catch on the redundant calls to getFilteredKeyboardNames(). I first wrote this app quickly on a commuter train, and once I got a working version I just went with it and never spent much time to go back over the code and tidy things up. I also have learned a lot more about shiny since.

I'm not sure why the deploy preview isn't including these edits. I thought it should.

As for the filter button, it wasn't immediately obvious to me what it did because the default setting was "show all", and it also kind of looks like an upload button:

Screen Shot 2022-01-04 at 6 18 28 AM

I think it might be clearer if you put some text, like Apply Filters? and have a Yes / No switch.

I also thought that you had intended to filter the table based on the selected boards (not just the filters). If you wanted to do that, it would probably be best to have two switches on the table page - one for the filters, and one for the selected boards.

Finally, since the filters will now apply to both pages, it would be nice if the filters could be edited from either page. This is where I think a switch to shinydashboard (or something similar) would be really nice, because then you could put the filters and the board selection boxes all in the left menu, which could be displayed or hidden from any page. Don't have to implement that right away, but it's a further motivation to explore that UI.

undefined-landmark commented 2 years ago

Good points, I agree. Maybe its better to just leave this pull request for now and implement this in a nicer way later on.

Last week I tried shinydashboard:

Screenshot 2022-01-04 at 13 16 17

To make sure I understand you correctly, would you put the filters in the fold away menu where I now have the three panes?

Another option would be to implement the datatable as a tab in the Keyboard Overlay box, so you can switch between the image overlay and the datatable views. Maybe that's also what you mean?

The keyboards pane could become a catalog/overview page with the same non filtered datatable and maybe also an overview with thumbnails of all the keyboards.

Lastly something I recently thought of, which is a bit of work, is a submission page. Most of the value of this dashboard depends on the underlying dataset and overlay images, any way to make the submission or addition of new keyboards easier would therefore be interesting. But for now shinydashboard and other features are a bit easier to implement :).

jhelvy commented 2 years ago

Yes I was thinking of putting the filters and check boxes in the fold away menu where you now have the three panes. So that menu will always be available and will control what is displayed on either the keyboard overlays or the datatable. While it looks like you can't use tabs in the header with shinydashboard, I believe you should be able to use a tabbed structure within the main content area. So the tabs would just be nested below the header (which is kind of annoying).

I'd leave the submission page idea alone for now. It's still a manual process to align all the switchboards in Illustrator, so submitting the necessary files via issues seems to be working. That said, I'm not planning on adding more in the future. It's a time-consuming process. I also will eventually run out of space to put all the labels, so at a certain point it's just not maintainable. I think a better version of this whole app would be to implement it in a totally different way (probably using Javascript) and allow the user to upload a svg that they can then dynamically resize themselves to create overlays.

undefined-landmark commented 2 years ago

I could give that a shot! Maybe it's possible to avoid panes/tabs altogether with that approach.

Automating the overlaying process, or outsourcing it to the person who wants to add it would be ideal. I'm not that well versed in Javascript but the idea sounds interesting.