Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add general popup menu for model browser #1995

Merged
merged 8 commits into from Jan 14, 2023
Merged

Add general popup menu for model browser #1995

merged 8 commits into from Jan 14, 2023

Conversation

amolenaar
Copy link
Member

@amolenaar amolenaar commented Jan 6, 2023

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

When you cleaned the whole model, there's no way to easily create new top-level packages without putting them in a diagram as well.

Issue Number: #1954

What is the new behavior?

When no element is selected, you can add top level diagrams and packages.

This allows to add top-level packages and diagrams from anywhere within the model browser (tree view).

You can now deselect the selected element in the model browser with Ctrl+mouse click.

image

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I considered adding it to the "create diagram" button in the header bar as well.

  • Renamed "tree component" on layout level as model_browser, as it's called in the docs.
  • Fixed issue where only new popover menu's were created and none were cleaned up
  • Remove GTK4 related code from Namespace component.
  • Break dependency from tree component/namespace on function in main window module.

When no element is selected, you can add toplevel diagrams and packages.
Reuse it, otherwise we're creating a lot of inactive popups
along the way.
Clean it when we need to create a new one.

This way the signal handling still works (we're not detaching the popup,
and break action propagation), and we're not creating a lot of inactive
popups.
For GTK4 we have the TreeComponent. Only need to support GTK3 for
Namespace component atm.
@github-actions github-actions bot added the python Pull requests that update Python code label Jan 6, 2023
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 6, 2023

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 1.01%.

Quality metrics Before After Change
Complexity 3.08 ⭐ 2.80 ⭐ -0.28 👍
Method Length 67.94 🙂 67.20 🙂 -0.74 👍
Working memory 4.55 ⭐ 4.28 ⭐ -0.27 👍
Quality 72.53% 🙂 73.54% 🙂 1.01% 👍
Other metrics Before After Change
Lines 1668 1666 -2
Changed files Quality Before Quality After Quality Change
gaphor/ui/mainwindow.py 71.94% 🙂 71.88% 🙂 -0.06% 👎
gaphor/ui/namespace.py 73.23% 🙂 76.77% ⭐ 3.54% 👍
gaphor/ui/namespaceview.py 49.69% 😞 55.49% 🙂 5.80% 👍
gaphor/ui/treecomponent.py 67.87% 🙂 67.20% 🙂 -0.67% 👎
gaphor/ui/tests/test_mainwindow.py 90.31% ⭐ 90.31% ⭐ 0.00%
gaphor/ui/tests/test_namespace.py 90.38% ⭐ 89.94% ⭐ -0.44% 👎
gaphor/ui/tests/test_treecomponent.py 85.70% ⭐ 85.73% ⭐ 0.03% 👍
tests/test_diagram_tools.py 71.30% 🙂 71.30% 🙂 0.00%

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
gaphor/ui/treecomponent.py list_item_factory_setup 12 🙂 510 ⛔ 33.41% 😞 Try splitting into smaller methods
gaphor/ui/namespaceview.py namespace_view 14 🙂 293 ⛔ 35.73% 😞 Try splitting into smaller methods
gaphor/ui/mainwindow.py MainWindow.open 3 ⭐ 388 ⛔ 47.74% 😞 Try splitting into smaller methods
gaphor/ui/treecomponent.py select_element 17 🙂 130 😞 49.65% 😞 Try splitting into smaller methods
gaphor/ui/namespace.py popup_model 9 🙂 213 ⛔ 8 🙂 52.95% 🙂 Try splitting into smaller methods

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

This is more in line with the documentation.
Breaks cycle where model browser components needed to import functions
from the main window module.
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like being able to right-click and add a new Package. The refactoring you did also is really nice, I like how the GTK4 and GTK3 code is split apart more for this component.

I noticed when I right-click and go to New Diagram, only the Class Diagram is displayed. Was this on purpose? Do we need diagrams that don't exist within any namespace / package?

@amolenaar
Copy link
Member Author

I noticed when I right-click and go to New Diagram, only the Class Diagram is displayed. Was this on purpose? Do we need diagrams that don't exist within any namespace / package?

Somehow the menu is not resizing to contain all diagram types.

Somehow the menu does not resize when the submenu is shown.
@amolenaar
Copy link
Member Author

amolenaar commented Jan 11, 2023

Not sure why the popup menu is not resizing.

Anyway, it may make sense to add all entries in the main menu anyhow:

image

@amolenaar amolenaar added this to the 2.16 milestone Jan 11, 2023
For GTK4 we have a different component.
@danyeaw danyeaw merged commit 8e1a73a into main Jan 14, 2023
@danyeaw danyeaw deleted the toplevel-package branch January 14, 2023 10:29
@danyeaw danyeaw added the feature A new feature label Jan 14, 2023
@danyeaw
Copy link
Member

danyeaw commented Jan 14, 2023

Thanks @amolenaar, nice improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants