napari / napari-plugin-manager

napari plugin manager to provide a graphical user interface for installing napari plugins.
https://napari.org/napari-plugin-manager
BSD 3-Clause "New" or "Revised" License
5 stars 5 forks source link

Reorganize list widget layout and fix issues #31

Closed goanpeca closed 2 months ago

goanpeca commented 6 months ago

Part of https://github.com/napari/napari-plugin-manager/issues/14


Comments from original issue that were fixed!

  1. Align plugin name and summary in the installed plugins section.

  2. Align installation info, Make a standard size? ( I know this button adjusts by the sizes on the drop down menus held within. Eliding may help here (see item below).

    image
  3. Ensure author names are aligned. (maybe misaligned only with many authors and in the presence of an update button?)

    image
  4. Ensure very long descriptions and author lists are not cut off.

    image
  5. Elide version numbers when long. When users have a plugin dev version installed the Installation info button & section is very long. We probably want to elide this

    image
  6. refactor code, one example is cleaning up the code that toggles which widgets are visible or not: https://github.com/napari/napari/pull/5198#discussion_r1028817033

  7. Set height of each item.

        if item.widget.install_info_button.isExpanded():
            item.widget.setFixedHeight(int(height * SCALE))
        else:
            item.widget.setFixedHeight(int(height / SCALE))
        item.setSizeHint(item.widget.size())

Current State with this PR

Screenshot 2024-07-10 at 2 24 58 AM Screenshot 2024-07-10 at 2 21 44 AM
jaimergp commented 3 months ago

@goanpeca What's the status here? Is this PR still active or should it be closed?

goanpeca commented 2 months ago

@goanpeca What's the status here? Is this PR still active or should it be closed?

Updated the PR, should be ready for review :)

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 94.69027% with 6 lines in your changes missing coverage. Please review.

Project coverage is 92.09%. Comparing base (7f90aa8) to head (35dd765).

Files Patch % Lines
napari_plugin_manager/qt_plugin_dialog.py 94.69% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #31 +/- ## ========================================== + Coverage 91.79% 92.09% +0.30% ========================================== Files 10 10 Lines 1669 1695 +26 ========================================== + Hits 1532 1561 +29 + Misses 137 134 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

goanpeca commented 2 months ago

@jaimergp added the comments :)

jaimergp commented 2 months ago

Thanks @goanpeca. @dalthviz, can you confirm the proposed changes address the items presented in https://github.com/napari/napari-plugin-manager/issues/14? Thanks!

dalthviz commented 2 months ago

Gave this a check on Windows and seems like the listed items from #14 that are being mentioned here get a fix here indeed :+1: However, not totally sure if the behavior of the layout after some interactions with the Installation Info widget and the way the version label is getting elided are expected. Just in case, a preview of how those things were behaving for me:

plugin_dialog_layout

jaimergp commented 2 months ago

Yea, looks like the extra button in the top panel (Update X.Y.Z) makes things behave a bit differently 🤔

goanpeca commented 2 months ago

Eliding indeed is no working as expected, definitely a bug on my side :)

Thanks for the review @dalthviz indeed weird behavior 😆 on resize, taking a look at it now.

psobolewskiPhD commented 2 months ago

Only thing I noticed was a flicker when you filter while it's still fetching:

image

Screenshot doesn't show it well, but type zarr into filter, the description of the plugin flickers.

goanpeca commented 2 months ago

@dalthviz added some dummy version to test the elide I think it is working, it just needs to be longer to be elided. Also added a fix for the resizing behavior bug you found. Could you check again :) ?

napari

@psobolewskiPhD I cannot seem to reproduce the flickering :(

dalthviz commented 2 months ago

Gave this another check and seems like the resize behavior I mentioned above is not reproducible anymore :+1: I still see the version string being cut in two lines from my side but seems like that is expected due to the version string length, if I'm understanding correctly, right?:

plugin_resize_elide_behavior

Also, related with the flickering behavior, I tried filtering things multiple times while the available plugins list seemed like still being populated. For comparison, couple of GIF showing the behaviors (changes here vs latest release - 0.1.0a2 vs latest main - 5027417):

Version Preview
This PR plugins_filter
0.1.0a2 plugins_filter_010a2
main (5027417) plugins_filter_main

I see with the changes here and with latest main a little freeze when filtering things, maybe is worthy to create an issue to track that? Related more specifically with the flickering, I think I'm able to see that behavior in all the versions that I checked. Maybe that is related with changes caused due to other plugins being loaded which change the width used for other fields like author and then cause the plugin description space to be shorter? Maybe creating an issue for that could be worthy too? 🤔

goanpeca commented 2 months ago

Hi @dalthviz thanks for the check again. Now I understand that the version is split in 2 lines... which is odd but, I know how to fix it :)

Will push a fix. Thanks again for the thorough reviews¡

goanpeca commented 2 months ago

So, the issue is that the eliding logic which is based for text, will split on given characters like a dot or a comma, or a dash etc. That is why you see the version split in 2 lines. So one "solution" would be to replace a normal dot by a different type of symbol. I tried a few and found one that could work :)

Thoughts @jaimergp, @dalthviz, @psobolewskiPhD ?

One dot leader ․

See https://www.amp-what.com/unicode/search/dot

Screenshot 2024-07-17 at 5 57 10 PM

Also created a new issue to track the flickering

jaimergp commented 2 months ago

So one "solution" would be to replace a normal dot by a different type of symbol. I tried a few and found one that could work :)

lmao hahah love it

goanpeca commented 2 months ago

This one is ready. Will continue opening smaller more concise issues to handle any remaining from https://github.com/napari/napari-plugin-manager/issues/14