JBPM 8915 : Stunner - Properties panel is being closed when switching assets or views #3129

inodeman · 2020-01-30T01:36:06Z

Hi @romartin can you review this PR? Will upload War in next comment

inodeman · 2020-01-30T01:39:35Z

Jenkins execute full downstream build

inodeman · 2020-01-30T03:05:06Z

War can be downloaded from https://drive.google.com/file/d/1LpSAYuPDwoL6Gzkm1xLEyMm7oSS1nmji/view?usp=sharing

inodeman · 2020-01-30T03:59:37Z

Hi @romartin war was uploaded, can you review it?

manstis · 2020-01-30T11:39:07Z

...va/org/kie/workbench/common/stunner/kogito/client/screens/DiagramEditorPropertiesScreen.java

@@ -59,6 +65,9 @@
    private final SessionManager clientSessionManager;
    private final Event<ChangeTitleWidgetEvent> changeTitleNotificationEvent;
    private final DiagramEditorScreenView view;
+    private final Event<ScreenPreMaximizedStateEvent> screenStateEvent;
+    private final Event<ScreenDiagramModelUnSelectedEvent> screenDiagramModelUnSelectedEventEvent;


Does this need to be called ...EventEvent?

My bad @manstis will change it

manstis · 2020-01-30T11:39:12Z

...va/org/kie/workbench/common/stunner/kogito/client/screens/DiagramEditorPropertiesScreen.java

@@ -59,6 +65,9 @@
    private final SessionManager clientSessionManager;
    private final Event<ChangeTitleWidgetEvent> changeTitleNotificationEvent;
    private final DiagramEditorScreenView view;
+    private final Event<ScreenPreMaximizedStateEvent> screenStateEvent;
+    private final Event<ScreenDiagramModelUnSelectedEvent> screenDiagramModelUnSelectedEventEvent;
+    private final Event<ScreenDiagramPropertiesSwitchingSessionsEvent> screenDiagramPropertiesSwitchingSessionsEventEvent;


Does this need to be called ...EventEvent?

My bad @manstis will change it

manstis · 2020-01-30T11:47:10Z

...main/java/org/kie/workbench/common/stunner/bpmn/project/client/editor/BPMNDiagramEditor.java

@@ -81,6 +88,10 @@

    private Consumer<Boolean> saveCallback;
    private boolean isMigrating = false;
+    private boolean useTimer = true;


Instead of adding test support with a flag it might be better to do:-

protected void deferOpenPropertiesDocks(final Command command) { new Timer() { @Override public void run() { command.execute(); } }.schedule(10); } ... @OnFocus public void onFocus() { final boolean shouldOpen = isPropertiesOpenedBeforeSwitchingSessions; super.doFocus(); if (shouldOpen) { deferOpenPropertiesDocks( ()-> openPropertiesDocks()); } }

Then in the test you can do:-

//I think this is the correct syntax! doAnswer(i-> ((Command) i.getParameters[0]).execute()).when(diagramEditor).deferOpenPropertiesDocks(any(Command.class)));

Great @manstis thanks for the tip, will change it

inodeman · 2020-01-30T17:33:31Z

Jenkins execute full downstream build

inodeman · 2020-01-30T20:31:15Z

Jenkins execute full downstream build

inodeman · 2020-01-30T20:31:43Z

Hi @LuboTerifaj I added the changes from the other PR about minimize panel, Explore panel should also work now on this PR

mbiarnes · 2020-01-31T11:35:26Z

Jenkins execute full downstream build

inodeman · 2020-01-31T20:25:18Z

Jenkins execute full downstream build

LuboTerifaj

Hello @inodeman ,

the issue is not fixed for every case.

If you have two assets with opened Properties or Explore diagram panel and you are switching between them, the panel is being closed.
You can switch between them either by using dropdown menu or using breadcrumb navigation panel.
Steps to reproduce:
1. Create process (or Case Definition or Case Management) "A"
2. Navigate to project using breadcrumbs.
3. Create process (or CD or CM) "B"
4. Open Explore diagram panel (it also applies to Properties panel in CM).
5. Open process "A" (using drop-down menu or breadcrumbs).
  Explore diagram panel should be open here (it also applies to Properties panel in CM).
6. Open back process "B".
  Explore diagram panel should be open here (it also applies to Properties panel in CM).
The issue when switching between Model, Overview and Documentation is not fixed either.
Steps to reproduce:
The first issue:
1. Ensure that Explore diagram panel is open.
2. Click on Overview or Documentation panel.
3. Click on Model
  Explore diagram should be open.
The second issue:
1. Ensure that Properties panel is open.
2. Click on Overview or Documentation panel.
3. Open Explore diagram panel.
4. Click on Model
  Explore diagram should be open.
There is also the issue with DMN editor. The Properties or Explore diagram panel is always closed when switching between assets. However, this is probably out of scope of this jira.

One more thing, can you please change the commit message since the https://issues.redhat.com/browse/JBPM-8997 is duplicate issue of https://issues.redhat.com/browse/JBPM-8915? I've already closed the former.

Thank you!

romartin · 2020-02-05T01:23:25Z

Please @inodeman this need rebase and rebuilds, otherwise we cannot proceed with the testing. thansk!

inodeman · 2020-02-05T02:52:14Z

Hi @romartin as requested by @LuboTerifaj I am making changes so that Case Definition Diagrams are also supported, the changes I initially made are for BPMN Diagrams only, also requested DMN Diagrams, so I need to add those 2 scenarios

romartin · 2020-02-05T03:11:36Z

ok thanks @inodeman , in fact it makes sense all editors behave the same way. But in case issues appears on other editors (CM/DMN) I would suggest creating a new ticket and merging this one, at least, which fixes the concrete issue requested.
Also please do the rebase and re-fire builds... Thanx!

inodeman · 2020-02-05T03:15:26Z

Very well @romartin will do, also I think it is a good idea that this PR be merged into this one, since some of the code is shared, this particular PR includes everything from #3127, what do you think?

inodeman · 2020-02-05T03:35:28Z

Jenkins execute full downstream build

inodeman · 2020-02-05T03:35:44Z

Hi @romartin rebased and resolved conflicts now

inodeman · 2020-02-05T16:39:54Z

Jenkins execute full downstream build

inodeman · 2020-02-06T00:51:41Z

Hi @LuboTerifaj artifacts generated, can you check this PR?

LuboTerifaj

Hello @inodeman , @romartin ,

Neither of the issues mentioned in my previous review is fixed.

See the summary below.

If you have two assets with opened Properties or Explore diagram panel and you are switching between them, the panel is being closed.

You can switch between them either by using dropdown menu or using breadcrumb navigation panel.
Steps to reproduce:
1. Create process (or Case Definition or Case Management) "A"
2. [Optional] Open Explore diagram panel.
3. Navigate to project using breadcrumbs.
4. Create process (or CD, CM) "B".
5. [Optional] Open Explore diagram panel.
6. Open process "A" (using drop-down menu or breadcrumbs).
  Neither Properties nor Explore diagram panel is open.
7. Open back process "B".
  Neither Properties nor Explore diagram panel is open.

The issues when switching between Model, Overview and Documentation are still not fixed.

Steps to reproduce:

The first issue:
1. Ensure that Explore diagram panel is open.
2. Click on Overview or Documentation panel.
3. Click on Model.
  Explore diagram is not open.
The second issue:
1. Ensure that Properties panel is open.
2. Click on Overview or Documentation panel.
3. Open Explore diagram panel.
4. Click on Model.
  Explore diagram is not open. Properties panel is open instead.

The commit message is not changed.

Kie code style is not applied in every file. See my comments inline.

LuboTerifaj · 2020-02-06T09:22:11Z

...kie/workbench/common/stunner/core/client/event/screen/ScreenDiagramModelUnSelectedEvent.java

+import org.kie.workbench.common.stunner.core.client.session.event.BaseSessionEvent;
+
+public class ScreenDiagramModelUnSelectedEvent extends BaseSessionEvent {
+    public ScreenDiagramModelUnSelectedEvent(final ClientSession session) {


Suggested change

public ScreenDiagramModelUnSelectedEvent(final ClientSession session) {

public ScreenDiagramModelUnSelectedEvent(final ClientSession session) {

LuboTerifaj · 2020-02-06T09:22:58Z

...h/common/stunner/core/client/event/screen/ScreenDiagramPropertiesSwitchingSessionsEvent.java

+import org.kie.workbench.common.stunner.core.client.session.event.BaseSessionEvent;
+
+public class ScreenDiagramPropertiesSwitchingSessionsEvent extends BaseSessionEvent {
+    public ScreenDiagramPropertiesSwitchingSessionsEvent(final ClientSession session) {


Suggested change

public ScreenDiagramPropertiesSwitchingSessionsEvent(final ClientSession session) {

public ScreenDiagramPropertiesSwitchingSessionsEvent(final ClientSession session) {

LuboTerifaj · 2020-02-06T09:23:54Z

.../org/kie/workbench/common/stunner/core/client/event/screen/ScreenPreMaximizedStateEvent.java

+
+    private final boolean isExplorerScreen;
+
+    public ScreenPreMaximizedStateEvent(final boolean isExplorerScreen ) {


Suggested change

public ScreenPreMaximizedStateEvent(final boolean isExplorerScreen ) {

public ScreenPreMaximizedStateEvent(final boolean isExplorerScreen) {

LuboTerifaj · 2020-02-06T09:26:25Z

...t/java/org/kie/workbench/common/stunner/client/widgets/palette/BS3PaletteWidgetImplTest.java

@@ -172,7 +172,7 @@ public void checkDestructionReleasesResources() {

    @Test
    public void checkOnScreenMaximisedDiagramEditor() {
-        final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true);
+        final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true );


Suggested change

final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true );

final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true);

inodeman · 2020-02-06T18:34:00Z

Jenkins execute full downstream build

inodeman · 2020-02-06T18:34:19Z

Jenkins retest please

inodeman · 2020-02-06T18:36:15Z

Hi @LuboTerifaj right now on the ticket fix works for BPMN files (Explore Panel not included, as it was not mentioned on ticket, will need additional time), for DMN and Case Management will need more time, you can check the issues with BPMN file and I can create jiras for Case and DMN or I can send the fix in a couple of days, let me know what you think? The code style, for some reason was not aplying, I rechecked it now, should be ok, also the commit message

LuboTerifaj · 2020-02-07T15:33:26Z

Hello @inodeman the issue is not fixed for Properties panel either.

See my previous comment for more details.

The first mentioned issue in that comment is also the part of steps to reproduce in JBPM-8915 jira.

Hence, the main issue is still not fixed.

inodeman · 2020-02-07T16:23:12Z

Hi @LuboTerifaj no problem, I will add Case Management and DMN, should be done early next week

romartin · 2020-02-13T21:51:00Z

@inodeman please do not forget this one. I know we're focused on kogito, but that was expected for the previous release and it's a customer request as well, so we shoud complete it anyway.
Thanks!

inodeman · 2020-02-13T22:31:52Z

no problem @romartin I can start on this tomorrow, just finished the quickstart examples its updates and the error issues creation

inodeman · 2020-02-14T22:01:56Z

Jenkins execute full downstream build

inodeman · 2020-02-14T22:05:31Z

Hi @LuboTerifaj , @romartin can you review this PR? @LuboTerifaj note this change only applies to bpmn models, you can test the cases when the user 1) maximizes and minimizes with both explore and properties panel, 2) switches views (Documentation, Overview and back to Diagram) for both the properties and explore, 3) Switching Sessions (Properties and Explore Panels), after you can verify how it is supposed to behave, I will go ahead and include DMN and Case Management models, I think this way, I can code it faster after I get your approval on desired behaviour, let me know

sonarcloud · 2020-02-15T00:55:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
7 Code Smells

64.2% Coverage
2.5% Duplication

LuboTerifaj · 2020-03-03T12:53:49Z

Jenkins execute full downstream build

LuboTerifaj · 2020-03-04T16:42:28Z

Jenkins execute full downstream build

LuboTerifaj

Hello @inodeman ,

switching between panels behaviour is not unified. The state of Properties/Explore diagram panel is not remembered for every case. There are two issues:
Steps to reproduce:

Ensure Properties (Explore diagram) panel is open in Model view.
Switch to Overview or Documentation.
Open Explore diagram (Properties) panel and close it back.
Open Model back.
Actual result: Explore diagram (Properties) panel is open.
Expected result: Properties (Explore diagram) panel is open.

Steps to reproduce:

Ensure Properties (Explore diagram) panel is closed in Model view.
Switch to Overview or Documentation.
Open Explore diagram (Properties) panel and close it back.
Open Model back.
Actual result: Explore diagram (Properties) panel is open.
Expected result: No panel is open.

Switching between assets is still not fixed. For more information see my review above.

Thanks!

inodeman · 2020-07-14T21:52:00Z

Hi @LuboTerifaj @romartin this PR is ready for review again.

This will apply to BPMN process and Case Definition Only, since as commented by @romartin there is no case in doing it for Case Management featture.

I fixed some issues in changing tabs, now it should behave as expected. Made a change, where you open Overview or Documentation and Open Properties or Explorer and you change to Documentation or Overview, they will remain open.

War can be found here https://drive.google.com/file/d/1i9Q_J8QxTODQ2c1Nj8z4Xt74U15Q3uFG/view?usp=sharing

romartin · 2020-07-29T22:33:13Z

@inodeman @LuboTerifaj see my comment... https://issues.redhat.com/browse/JBPM-8915?focusedCommentId=14324800&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14324800

… assets or views JBPM-8915 : Stunner - Properties panel is being closed when switching assets or views Added Logs JBPM-8915 : Stunner - Properties panel is being closed when switching assets or views Fixed Maximize / Switching Assets Views

inodeman · 2021-02-05T19:40:51Z

Jenkins retest this

inodeman · 2021-02-05T19:41:05Z

jenkins do fdb

sonarcloud · 2021-02-05T23:20:38Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

53.4% Coverage
2.5% Duplication

… assets or views

inodeman · 2021-02-08T21:34:39Z

Jenkins retest this

inodeman · 2021-02-08T21:34:51Z

jenkins do fdb

inodeman · 2021-02-11T19:34:39Z

Jenkins retest this

inodeman · 2021-02-11T19:35:10Z

jenkins do fdb

romartin · 2021-03-29T11:39:19Z

Closing - out of date and issue closed

manstis reviewed Jan 30, 2020

View reviewed changes

romartin requested review from romartin and LuboTerifaj January 30, 2020 16:03

inodeman force-pushed the JBPM-8997 branch from 9c1f028 to 1964258 Compare January 30, 2020 17:33

LuboTerifaj suggested changes Feb 4, 2020

View reviewed changes

inodeman force-pushed the JBPM-8997 branch from 3c4c6ba to eef3eff Compare February 5, 2020 03:35

LuboTerifaj suggested changes Feb 6, 2020

View reviewed changes

inodeman changed the title ~~JBPM 8997 : Stunner - Properties panel is being closed when switching assets or views~~ JBPM 8915 : Stunner - Properties panel is being closed when switching assets or views Feb 6, 2020

inodeman force-pushed the JBPM-8997 branch from eef3eff to a7aacb5 Compare February 6, 2020 18:32

inodeman force-pushed the JBPM-8997 branch from a7aacb5 to d8fc77a Compare February 14, 2020 22:01

LuboTerifaj suggested changes Mar 5, 2020

View reviewed changes

inodeman force-pushed the JBPM-8997 branch from d8fc77a to 274f9ef Compare July 14, 2020 21:46

inodeman force-pushed the JBPM-8997 branch from 274f9ef to fa27502 Compare February 5, 2021 19:40

JBPM 8915 : Stunner - Properties panel is being closed when switching…

21937a0

… assets or views

romartin closed this Mar 29, 2021

	public ScreenDiagramModelUnSelectedEvent(final ClientSession session) {

	public ScreenDiagramModelUnSelectedEvent(final ClientSession session) {

	public ScreenDiagramPropertiesSwitchingSessionsEvent(final ClientSession session) {

	public ScreenDiagramPropertiesSwitchingSessionsEvent(final ClientSession session) {


		private final boolean isExplorerScreen;

		public ScreenPreMaximizedStateEvent(final boolean isExplorerScreen ) {

	final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true );
	final ScreenMaximizedEvent event = new ScreenMaximizedEvent(true);

JBPM 8915 : Stunner - Properties panel is being closed when switching assets or views #3129

JBPM 8915 : Stunner - Properties panel is being closed when switching assets or views #3129

Conversation

inodeman commented Jan 30, 2020

inodeman commented Jan 30, 2020

inodeman commented Jan 30, 2020

inodeman commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inodeman commented Jan 30, 2020

inodeman commented Jan 30, 2020

inodeman commented Jan 30, 2020

mbiarnes commented Jan 31, 2020

inodeman commented Jan 31, 2020

LuboTerifaj left a comment

Choose a reason for hiding this comment

romartin commented Feb 5, 2020

inodeman commented Feb 5, 2020 • edited

romartin commented Feb 5, 2020

inodeman commented Feb 5, 2020

inodeman commented Feb 5, 2020

inodeman commented Feb 5, 2020

inodeman commented Feb 5, 2020

inodeman commented Feb 6, 2020

LuboTerifaj left a comment

Choose a reason for hiding this comment

If you have two assets with opened Properties or Explore diagram panel and you are switching between them, the panel is being closed.

The issues when switching between Model, Overview and Documentation are still not fixed.

The commit message is not changed.

Kie code style is not applied in every file. See my comments inline.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inodeman commented Feb 6, 2020

inodeman commented Feb 6, 2020

inodeman commented Feb 6, 2020 • edited

LuboTerifaj commented Feb 7, 2020

inodeman commented Feb 7, 2020

romartin commented Feb 13, 2020

inodeman commented Feb 13, 2020

inodeman commented Feb 14, 2020

inodeman commented Feb 14, 2020

sonarcloud bot commented Feb 15, 2020

LuboTerifaj commented Mar 3, 2020

LuboTerifaj commented Mar 4, 2020

LuboTerifaj left a comment

Choose a reason for hiding this comment

inodeman commented Jul 14, 2020

romartin commented Jul 29, 2020

inodeman commented Feb 5, 2021

inodeman commented Feb 5, 2021

sonarcloud bot commented Feb 5, 2021

inodeman commented Feb 8, 2021

inodeman commented Feb 8, 2021

inodeman commented Feb 11, 2021

inodeman commented Feb 11, 2021

romartin commented Mar 29, 2021

inodeman commented Feb 5, 2020 •

edited

inodeman commented Feb 6, 2020 •

edited