search

IDAES / idaes-ui

User interfaces for IDAES
Other
1 stars 5 forks source link

For Issue 46: Fixed the malfunction of the Zoom button when the fv display is in full-screen mode. #47

Closed CopyDemon closed 6 months ago

CopyDemon commented 7 months ago

Fixes Fixed the malfunction of the Zoom button when the fv display is in full-screen mode.

Proposed changes:

How to test:

Build environment and start UI

  1. Create a conda environment with python >= 3.8
  2. Switch to the environment
  3. Pull PR branch #46 and go to the project root folder run: pip install . pip install -r requirements-dev.txt
  4. In jupyter notebook create your flowsheet and store the flowsheet instance in a variable
  5. Start UI by call m.fs.visualize("flowsheet_name")

Test UI

Those previous steps should bring flowsheet visualizer up in a browser window.

  1. Click on full-window button on flowsheet panel header image
  2. When you see that the flowsheet panel has enlarged, click on the zoom in and zoom out buttons to test if they work properly. image

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.
CopyDemon commented 7 months ago

@dangunter ready for testing

dangunter commented 6 months ago

Working from your fork/main, I see the fix, but I have a request because the current behavior is not intuitive. When I click on full-screen, what it's really doing afaict is minimizing the "stream table" into a button up at the top. I can undo it by clicking on that button again. But, it should really be possible to also undo it by clicking on the "full screen" button again. In other words, the normal behavior of such buttons is to act as a toggle between the two modes.

CopyDemon commented 6 months ago

Working from your fork/main, I see the fix, but I have a request because the current behavior is not intuitive. When I click on full-screen, what it's really doing afaict is minimizing the "stream table" into a button up at the top. I can undo it by clicking on that button again. But, it should really be possible to also undo it by clicking on the "full screen" button again. In other words, the normal behavior of such buttons is to act as a toggle between the two modes.

@dangunter I understand your request and think it's a good idea. May I create a new pull request to address that issue?

dangunter commented 6 months ago

@CopyDemon I think you should fix it here, since it's the same button. Also I noticed another bug. If you click the "maximize" button, then click the zoom out (minus), the diagram disappears, and zooming back in (plus) does nothing.

dangunter commented 6 months ago

If it makes more sense to do this in the newer UI because it has the different layout engine, I guess I understand. But the more I use it, the more I think we don't want a "maximize" button at all, just a consistent "minimize" and "restore" functionality for any of the panels. The usual way to do this in an IDE like PyCharm, VS Code, or Jupyter Lab is to add a minimize button on the panel and add icons for each possible panel in a gutter on the left, so if you click on that icon it restores a minimized panel. However the current style of putting them as buttons along a top "gutter" would also work, with the advantage that full names don't require memory of icon meanings.

CopyDemon commented 6 months ago

@dangunter I removed the maximize button from the Flowsheet component header and added a minimize button to the stream table header. Now, both the Flowsheet and stream table only have minimize buttons.

Please help me review the new changes.

CopyDemon commented 6 months ago

@dangunter Fixed the issue where repeatedly minimizing the panel caused the zoom in and zoom out values to become excessively large. The root of this problem was that minimizing the panel triggered a React state change and re-rendered the DOM. Each DOM re-render initiated a new Toolbar class instance, which then assigned duplicate event listeners to the zoom in and zoom out buttons.