Project

General

Profile

Actions

Bug #18207

closed

Workbench2 is not clearing the project content when switching

Added by Daniel Kutyła over 2 years ago. Updated about 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Story points:
-
Release relationship:
Auto

Description

This is another cosmetic issue. When switching to a different project the table view content of the project is cleared late. This causes the project content looking incorrect.


Files


Subtasks 1 (1 open0 closed)

Task #18225: Review 18207-Workbench2-is-not-clearing-the-project-content-when-switchingIn ProgressDaniel Kutyła11/04/2021Actions

Related issues

Related to Arvados - Bug #16951: Can refresh listing when opening context menu, resulting in wrong item being operated on.ResolvedLucas Di Pentima01/18/2022Actions
Related to Arvados - Bug #18769: Test & confirm no stale data or flickering during data table refreshesResolvedLucas Di Pentima02/17/2022Actions
Related to Arvados - Bug #18972: "All processes" view reloading flickerResolvedLucas Di Pentima04/08/2022Actions
Actions #1

Updated by Peter Amstutz over 2 years ago

  • Target version set to 2021-10-13 sprint
Actions #2

Updated by Peter Amstutz over 2 years ago

  • Assigned To set to Daniel Kutyła
Actions #3

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-10-13 sprint to 2021-10-27 sprint
Actions #4

Updated by Daniel Kutyła over 2 years ago

  • Status changed from New to In Progress
Actions #5

Updated by Daniel Kutyła over 2 years ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/df0c0d462a52003cd722d25520cd7a4ad6583c57
Test run: developer-tests-workbench2: #503
Branch: 18207-Workbench2-is-not-clearing-the-project-content-when-switching

Project switch should trigger loader

Actions #6

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-10-27 sprint to 2021-11-10 sprint
Actions #7

Updated by Lucas Di Pentima over 2 years ago

The fix seems to work as expected: the data explorer component immediately shows an "in progress" indicator when changing projects, but I think the code updates make the ProjectPanel component a bit more coupled with external variables and/or state.

We already have an app-wide "in progress" indicator that is being used, and also state.dataExplorer?.projectPanel.items could be reset (or set to undefined?) on every panel change, what do you think? I feel that duplicating the previous query results & browser's href shouldn't be necessary and makes things a little more complicated to manage.

Related to this, the "Shared with me" at the left side bar doesn't seem to be using this new change, this may also be a symptom of needing to fix the problem in a deeper level (at DataExplorer's level for example) so any part of the app could benefit.

Actions #8

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-11-10 sprint to 2021-11-24 sprint
Actions #9

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-11-24 sprint to 2021-12-08 sprint
Actions #10

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-12-08 sprint to 2022-01-05 sprint
Actions #11

Updated by Daniel Kutyła over 2 years ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/394ebdfd13fe40a7096f484c46a353d2537f4c9a
Test run: developer-tests-workbench2: #551
Branch: 18207-Workbench2-is-not-clearing-the-project-content-when-switching

Merged main

Actions #12

Updated by Lucas Di Pentima over 2 years ago

Lucas Di Pentima wrote:

The fix seems to work as expected: the data explorer component immediately shows an "in progress" indicator when changing projects, but I think the code updates make the ProjectPanel component a bit more coupled with external variables and/or state.

We already have an app-wide "in progress" indicator that is being used, and also state.dataExplorer?.projectPanel.items could be reset (or set to undefined?) on every panel change, what do you think? I feel that duplicating the previous query results & browser's href shouldn't be necessary and makes things a little more complicated to manage.

Related to this, the "Shared with me" at the left side bar doesn't seem to be using this new change, this may also be a symptom of needing to fix the problem in a deeper level (at DataExplorer's level for example) so any part of the app could benefit.

The updates made in arvados-workbench2|df0c0d4 are just a main merge. Could you explain here why you dismissed my 3 comments? I think it's important for historic reasons that the discussion about the change reasonings is kept in the ticket.

Actions #13

Updated by Peter Amstutz about 2 years ago

  • Target version changed from 2022-01-05 sprint to 2022-01-19 sprint
Actions #14

Updated by Daniel Kutyła about 2 years ago

I merged this code before going AFK for the longer time, so it can be deployed within the next release as I was asked about this fix. I wrote explaination in the private message to you.

Actions #15

Updated by Lucas Di Pentima about 2 years ago

Daniel Kutyła wrote:

I merged this code before going AFK for the longer time, so it can be deployed within the next release as I was asked about this fix. I wrote explaination in the private message to you.

Well, I don't think it's ready to be merged yet, so going AFK isn't enough to make it ready.

As I mentioned #18207-7 and #18207-12, there're still some issues, being one of them another instance of this same bug, maybe you overlooked my comments, or perhaps you decided they aren't worth addressing.

If the latter is the case, please explain the reasons why. We're an open-source project, and we should record all discussions related to it in public because it'll surely be helpful to have the reasons documented for future reference.

Actions #16

Updated by Daniel Kutyła about 2 years ago

Hi Lucas
I tried to reuse this loader
that we use commonly
unfortunately it is way to hard
because it is triggered by the services
and thus it is very unpredictable
even though I remove some loaders it is not possible to prevent for new one to happen (because of the ongoing network reuquests)
I rebased branch with the newest main
so if this is not crucial I would opt for merging this one
and maybe we could create a ticket to change the way loader is shown

Actions #17

Updated by Lucas Di Pentima about 2 years ago

Lucas Di Pentima wrote:

Related to this, the "Shared with me" at the left side bar doesn't seem to be using this new change, this may also be a symptom of needing to fix the problem in a deeper level (at DataExplorer's level for example) so any part of the app could benefit.

So how about this? Please make sure the "Shared with me" listing behaves the same way as the rest. As I mentioned on my first comment, the fact that it isn't showing the progress indicator is a clue that there might be code duplication, so it would be great to eliminate that.

Actions #18

Updated by Daniel Kutyła about 2 years ago

I check shared with me and it seems to be working as other only the main project seems to be not showing the loader but for me it is loading reallyfast that is why loader is not shown

Actions #19

Updated by Lucas Di Pentima about 2 years ago

This is a video created from using your branch against ce8i5, you can see that the "Shared with me" UI doesn't use the new progress indicator.

Personally, I don't like that we have duplicated progress indicators working at the same time, but if we must, at least we should have consistency.

Actions #20

Updated by Daniel Kutyła about 2 years ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/f528cc85e7687e1d899860c4226984bd067c5bd3
Test run: developer-tests-workbench2: #560
Branch: 18207-Workbench2-is-not-clearing-the-project-content-when-switching

Removed duplicated loader to make the UI consistent, but kept the check with 'null' as a value when application is 'working' in order to force React to re-render and thus remove old project contents from the screen

Actions #21

Updated by Lucas Di Pentima about 2 years ago

For UI consistency, if you want, you could add some images and messages similar to the ones we show on empty projects but notify the user that they should be getting the data soon, as it's being loaded.
The 'null' option is acceptable by me, too; I just think it would be nicer to add some content to the blank space.
About my previous comment, the UI inconsistency I was referring to is that the "Shared with me" panel doesn't behave the same way as the project panel in the sense that its contents don't get cleared before loading the new data.
I've looked at the code, and I think that passing the loading prop to DataExplorer in src/views/shared-with-me-panel/shared-with-me-panel.tsx would do the trick.

Actions #22

Updated by Daniel Kutyła about 2 years ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/89a4eb90454f12be703f711279d0867781013631
Test run: developer-tests-workbench2: #562
Branch: 18207-Workbench2-is-not-clearing-the-project-content-when-switching

Added common icon, removed code duplication

Actions #23

Updated by Lucas Di Pentima about 2 years ago

Updates at arvados-workbench2|facccf58

  • The 'Shared with me' issue from #18207-19 kept existing, and I also detected that the 'Groups' section was suffering from the same problem.
  • To avoid further back and forth, I took the liberty to fix those, and I also replaced the "folder" icon with another more generic one, as the DataTable component is used not only on project listings, but on other object type listings.
  • If you agree with the changes, I think this is ready to be merged.
Actions #24

Updated by Daniel Kutyła about 2 years ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #25

Updated by Peter Amstutz about 2 years ago

  • Related to Bug #16951: Can refresh listing when opening context menu, resulting in wrong item being operated on. added
Actions #26

Updated by Peter Amstutz about 2 years ago

  • Related to Bug #18769: Test & confirm no stale data or flickering during data table refreshes added
Actions #27

Updated by Peter Amstutz about 2 years ago

  • Release set to 46
Actions #28

Updated by Lucas Di Pentima almost 2 years ago

  • Related to Bug #18972: "All processes" view reloading flicker added
Actions

Also available in: Atom PDF