glotzerlab / signac-dashboard

Rapidly visualize signac projects through a customizable dashboard interface.
https://signac.io
BSD 3-Clause "New" or "Revised" License
16 stars 6 forks source link

Fix/allow multiple notes #182

Open cbkerr opened 1 year ago

cbkerr commented 1 year ago

Description

Uses Blueprints to differentiate between module endpoints in the Notes module. I also consider this a prototype to use Blueprints for all modules so that you can enable multiple of them.

There is still a problem where it runs the endpoint notes_update as many times as there are instances of the Notes module. This problem relates to registering the module assets. If I comment out the part about dashboard.register_module_asset(...), it only runs the endpoint once, but you have to manually go back to the page because you end up on the URL of the action.

Motivation and Context

Resolves #74

image

Checklist:

codecov[bot] commented 1 year ago

Codecov Report

Merging #182 (e462385) into main (81981c1) will increase coverage by 0.10%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #182      +/-   ##
==========================================
+ Coverage   69.98%   70.09%   +0.10%     
==========================================
  Files          21       21              
  Lines         883      886       +3     
  Branches      159      159              
==========================================
+ Hits          618      621       +3     
  Misses        220      220              
  Partials       45       45              
Files Changed Coverage Δ
signac_dashboard/modules/notes.py 71.42% <100.00%> (+2.67%) :arrow_up:
bdice commented 1 year ago

@cbkerr It looks like this PR may need to be updated a bit. It appears to have commits from #181. It might be that this is in a problematic state due to the master/main rename, and changing the draft state or target branch may force it to refresh. You could also rebase on main and force-push, I think. Let me know when this is ready for review, when you open the draft.

edit: Seems like maybe this fixed itself?

bdice commented 1 year ago

There is still a problem where it runs the endpoint notes_update as many times as there are instances of the Notes module. This problem relates to registering the module assets.

I'm a bit concerned about merging this PR with a design flaw like this. @cbkerr Do you see a way forward for this?

bdice commented 12 months ago

@cbkerr Ping - just checking to see if you have thoughts about the registration problem. Do you think this is a blocker? Or should we merge this PR regardless, and file an issue?

bdice commented 11 months ago

Removing review requests due to inactivity.