8075 – Unnatural expanding UI when navigating lists in tlwebadm

Bug 8075 - Unnatural expanding UI when navigating lists in tlwebadm

Summary: Unnatural expanding UI when navigating lists in tlwebadm

Status:	CLOSED FIXED

Alias:	None

Product:	ThinLinc
Classification:	Unclassified
Component:	Web Administration (show other bugs)
Version:	trunk
Hardware:	Other Other

Importance:	P2 Normal
Target Milestone:	4.15.0
Assignee:	Tobias

URL:
Keywords:	aleze_tester, relnotes

Depends on:
Blocks:	5900
	Show dependency tree / graph

Reported:	2023-01-20 14:02 CET by Tobias
Modified:	2023-04-26 13:12 CEST (History)
CC List:	3 users (show)

See Also:	7909 8080 8106 8113 8084 8090 8119 8103
Acceptance Criteria:	* Items in tables that, when edited or added, were previously expanding the table should be replaced by a popup. * No part of the popup should be inaccessible — if larger than the viewport, we should be able to scroll. * The popup should automatically scroll to show any errors that appear. * It shouldn't be possible to interact with the page behind the popup, without closing the popup first. * It should always be possible to close the popup. * The title of the popup should properly reflect its contents.

Attachments
Add an attachment (proposed patch, testcase, etc.)

Description Tobias cendio

2023-01-20 14:02:23 CET

Inspecting/editing lists in tlwebadm results in a local allocation of space inside the list, for instance under Profiles / Profile List.

This has quite a jumpy feel to it, and it would feel smoother if the content appeared in another way.

Comment 56 Frida Flodin cendio

2023-02-06 14:13:40 CET

Remember to test subcluster with name "NewSubcluster". It could cause inconsistent behavior with the popup if bug 8084 is not fixed yet.

Comment 83 Frida Flodin cendio

2023-02-22 16:00:46 CET

I have looked over the issue when you want to see details for an item in a popup, but the item is gone from the backend.

For example, you are at the sessions page and have not refreshed the page in a while and you click on a session. What should happen if this session no longer exists? 

After some discussion, we have an idea of how this should be handled:
1. You should still get at popup
2. There should be an informative error message.


I have looked into how this has been handled before, in 4.14.0. On most pages nothing happens. Clicking on a non-existing item looks like a refresh for the user, the item you clicked is gone and no error message. 

Some pages were different:
/desktop/menustructure: Is the only page that handled this well. You get a nice error message that "No such menu".

/desktop/appgroups: Start to create a new item with the same name as the one you clicked.

/desktop/manapplications: crashes with:
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30] ----------------------------------------
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30] Traceback (most recent call last):
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/server.py", line 162, in post_or_get
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]     OooO0O00o0 , OOo = getattr ( IIiII11I , action ) ( I11ii1IiIii , query , OooO0O00o0 )
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/main.py", line 130, in do_GET
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]     self . _GET_METHODS . get ( page_name , self . error_404 ) ( query ) )
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/desktop.py", line 85, in manapplications
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]     return self . _applications ( query , sysapps = False )
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/desktop.py", line 126, in _applications
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]     i1iiiiIIIiIi [ 'details' ] = self . _get_application_details ( query ,
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/desktop.py", line 336, in _get_application_details
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30]     i1i1Ii = ii1Iii . getExecArgv ( )
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30] AttributeError: 'NoneType' object has no attribute 'getExecArgv'
> 2023-02-21 11:47:22 ERROR tlwebadm[4847]: [::ffff:10.47.1.30] ----------------------------------------



When I test build 3083 I get mostly the same result. The exceptions are:
/vsm/subcluster: Starts to create a new subcluster with the name you clicked. Like /desktop/appgroups does.

/status/sessions: Works as we want, we get an error message saying "Session is not available" in a popup.

/status/load: Did not have a detailed view before, so this was not an issue. But now, the page crashes with:
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1] ----------------------------------------
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1] Exception happened during processing of request from ('::1', 41590, 0, 0)
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1] Traceback (most recent call last):
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/forkingserver.py", line 62, in process_request
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self . finish_request ( request , client_address )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/httpserver.py", line 407, in finish_request
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     super ( ) . finish_request ( request , client_address )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/tlstunnel.py", line 71, in finish_request
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self . TLSRequestHandlerClass ( request , client_address , self )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/httpserver.py", line 78, in __init__
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     super ( ) . __init__ ( request , client_address , server )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/usr/lib64/python3.10/socketserver.py", line 747, in __init__
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self.handle()
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/httpserver.py", line 380, in handle
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     super ( ) . handle ( )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/usr/lib64/python3.10/http/server.py", line 433, in handle
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self.handle_one_request()
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/httpserver.py", line 155, in handle_one_request
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     super ( ) . handle_one_request ( )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/usr/lib64/python3.10/http/server.py", line 421, in handle_one_request
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     method()
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/server.py", line 161, in do_GET
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self . post_or_get ( "do_GET" , II1iiii )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/server.py", line 175, in post_or_get
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     OooO0O00o0 , OOo = getattr ( IIiII11I , action ) ( I11ii1IiIii , query , OooO0O00o0 )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/main.py", line 133, in do_GET
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     self . _GET_METHODS . get ( page_name , self . error_404 ) ( query ) )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]   File "/opt/thinlinc/modules/thinlinc/tlwebadm/status.py", line 161, in load
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1]     O0o00OOOoo00 [ oO0O0oOOo0Oo ] )
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1] KeyError: '123'
> 2023-02-22 14:24:35 ERROR tlwebadm[569878]: [::1] ----------------------------------------

Comment 122 Samuel Mannehed cendio

2023-03-17 16:54:52 CET

All functional issues relating to the popup should now be resolved. Marking as resolved.

Comment 123 Samuel Mannehed cendio

2023-03-17 17:08:35 CET

The issue with the scroll position below the popup was broken out to bug 8119.

Comment 124 Tobias cendio

2023-03-22 09:22:28 CET

There are some issues concerning saving popup input listed in bug 8103. These issues primarily concern saving input followed by errors. Adding to that, the popup title is lost on pages

* Profiles/Profile List
* Locations/Terminals

when attempting to update an existing item with an empty name. Although one remains in the popup, the title has been corrupted and it looks broken.

Comment 125 Tobias cendio

2023-03-22 09:50:23 CET

Correction to comment 124: popup titles may currently disappear on pages 

* Desktop Customizer/Applications (Manual)
* Locations/Terminals

Reopening to fix this.

Comment 129 Tobias cendio

2023-03-23 16:33:37 CET

Concerning comment 125:

Popup titles should be fixed now for Locations/Terminals. Moreover, fallbacks have been added to ensure at least no missing popup titles.

Comment 130 Alexander Zeijlon cendio

2023-04-04 17:01:15 CEST

Reopen with comments after verification.

1. Pop-up titles do not always properly reflect pop-up content.
   - An example of this is in Status⇾Load, where the pop-up's title is
     the agent's name. The default agent is called "127.0.0.1" which
     is not indicative of type of content.
2. The size of the pop-up div that stretches the full width and height
   of the viewport uses percentage.
   - This works, but I think a more robust solution would be to use the
     units vh and vw, since this would make it more clear that we want
     this div to always fill the viewport.
3. Error messages are not centered in the pop-up.
   - CSS styling indicates that centered messages is the desired behavior.
4. The pop-up functions are not documented in tllib.
   - This would be good to have especially for the begin-function since it
     calls other tllib-functions that add functionality outside the pop-up
     context.
5. The min-width on #popup_window feels misplaced.
   - The funcitonality to be able to show content in a pop-up in front of
     the page should not be coupled with styling the content inside the
     pop-up.

Comment 131 Alexander Zeijlon cendio

2023-04-05 08:17:05 CEST

Continuation on comment 130:

6. The fade in/out animation on the pop-up should not trigger when the
   pop-up is open between page reloads.
   - For example, when clicking save causes an error such that we stay
     in the pop-up. Effectively, the pop-up has not either closed or
     opened when this happens, hence, we should not animate it.

Comment 132 Samuel Mannehed cendio

2023-04-05 09:23:51 CEST

7. The fade out animation on the pop-up should trigger when pressing "save".

Comment 133 Samuel Mannehed cendio

2023-04-05 09:32:15 CEST

(In reply to Alexander Zeijlon from comment #130)
> 2. The size of the pop-up div that stretches the full width and height
>    of the viewport uses percentage.
>    - This works, but I think a more robust solution would be to use the
>      units vh and vw, since this would make it more clear that we want
>      this div to always fill the viewport.

This is intentional, see commit r39675.

> 3. Error messages are not centered in the pop-up.
>    - CSS styling indicates that centered messages is the desired behavior.

Fixed in commit r40031.

Comment 136 Samuel Mannehed cendio

2023-04-06 16:19:19 CEST

(In reply to Alexander Zeijlon from comment #131)
> Continuation on comment 130:
> 
> 6. The fade in/out animation on the pop-up should not trigger when the
>    pop-up is open between page reloads.
>    - For example, when clicking save causes an error such that we stay
>      in the pop-up. Effectively, the pop-up has not either closed or
>      opened when this happens, hence, we should not animate it.

One available trigger for when the fade-in animation should trigger is to check if “details” are part of the search parameters in the URL. JavaScript can check this, but one obstacle is that normally JavaScript runs before the DOM is completely loaded. Waiting for the entire DOM to finish loading is also too late, since it can result in ugly “flashes” since the page doesn't render instantly. Such flashes are very jarring to the eye and is something we want to avoid.

We investigated the “DOMContentLoaded” event, but it didn't trigger early enough. We also attempted to add listeners to “animationstart” and as quickly as possible abort the animation, but this also resulted in briefly visible flashes.

The easiest solution seems to be to add a <script> tag running this small bit of JavaScript directly below the element in question in the HTML. Due to how browsers parse HTML, this means the relevant element is available in the DOM and can be accessed by the script, and also that we can apply changes quickly before the element is rendered.

(In reply to Samuel Mannehed from comment #132)
> 7. The fade out animation on the pop-up should trigger when pressing "save".

This was broken out to bug 8136, see more details on that bug.

Comment 139 Samuel Mannehed cendio

2023-04-06 16:23:32 CEST

(In reply to Alexander Zeijlon from comment #130)
> Reopen with comments after verification.
> 
> 1. Pop-up titles do not always properly reflect pop-up content.
>    - An example of this is in Status⇾Load, where the pop-up's title is
>      the agent's name. The default agent is called "127.0.0.1" which
>      is not indicative of type of content.
This was fixed in commit r40042.

> 4. The pop-up functions are not documented in tllib.
>    - This would be good to have especially for the begin-function since it
>      calls other tllib-functions that add functionality outside the pop-up
>      context.
This was fixed in commit r40044.

> 6. The fade in/out animation on the pop-up should not trigger when the
>    pop-up is open between page reloads.
>    - For example, when clicking save causes an error such that we stay
>      in the pop-up. Effectively, the pop-up has not either closed or
>      opened when this happens, hence, we should not animate it.
This was fixed in commit r40046.

> 5. The min-width on #popup_window feels misplaced.
>    - The funcitonality to be able to show content in a pop-up in front of
>      the page should not be coupled with styling the content inside the
>      pop-up.
This remains. One idea is to move the min-width to #popup_contents.

Comment 141 Tobias cendio

2023-04-11 14:17:17 CEST

(In reply to Samuel Mannehed from comment #139)
> > 5. The min-width on #popup_window feels misplaced.
> >    - The funcitonality to be able to show content in a pop-up in front of
> >      the page should not be coupled with styling the content inside the
> >      pop-up.
> This remains. One idea is to move the min-width to #popup_contents.

Fixed in commit r40050. The min-width attribute was moved to the popup content.

Comment 142 Tobias cendio

2023-04-11 14:45:07 CEST

The problems highlighted in comment 130 to comment 132 have been addressed in comment 133, comment 136, comment 139, and comment 141.

This bug should be resolved now.

Comment 143 Alexander Zeijlon cendio

2023-04-12 13:23:45 CEST

The issues discussed in comment 133, comment 139, and comment 141 have now been addressed, and their corresponding commits have been verified to now comply with the acceptance criteria in this bug.

The issue regarding fade in/out animations, discussed in Comment 136 has been broken out into bug 8136.

This bug is therefore resolved and can be closed.

Comment 144 Samuel Mannehed cendio

2023-04-19 13:47:30 CEST

From bug 8136 comment 1: 

> Additionally, there is no fade in animation on some pages when buttons on the main page for adding new entries are clicked.
> 
> > |Button               |Animation|
> > |"Add new subcluster" |Yes      |
> > |"Add new profile"    |No       |
> > |"Add new location"   |No       |
> > |"Add new terminal"   |No       |
> > |"Add new group"      |Yes      |
> > |"Add new application"|No       |
> > |"Add new menu"       |No       |

This was meant to be fixed as part of commit r40046. It seems like a minor fix, reopening.

Comment 148 Samuel Mannehed cendio

2023-04-19 13:52:27 CEST

(In reply to Samuel Mannehed from comment #144)
> From bug 8136 comment 1: 
> 
> > Additionally, there is no fade in animation on some pages when buttons on the main page for adding new entries are clicked.
> > 
> > > |Button               |Animation|
> > > |"Add new subcluster" |Yes      |
> > > |"Add new profile"    |No       |
> > > |"Add new location"   |No       |
> > > |"Add new terminal"   |No       |
> > > |"Add new group"      |Yes      |
> > > |"Add new application"|No       |
> > > |"Add new menu"       |No       |
> 
> This was meant to be fixed as part of commit r40046. It seems like a minor
> fix, reopening.

Fixed now. We should now have proper fade-in animations in the popup everywhere. The only remaining issues are w.r.t fade-out (bug 8136).

Comment 149 Samuel Mannehed cendio

2023-04-20 10:24:26 CEST

A slight logic modification was made to how the backend for the Menustructure page works. Thus, aside from testing that the fade-in animations behave as intended, we should verify that adding/editing/saving/deleting items (and associated errors) still work on the Menustructure page.

Comment 151 Alexander Zeijlon cendio

2023-04-26 13:12:04 CEST

The missing fade in animations mentioned in comment 144 have now been addressed and are verified to be working, this also includes the "Add sub menu" links on the Menu Structure page which were not mentioned in comment 144.

Furthermore, the updated logic for the Menu Structure page, addressed in comment 149 have been verified and tested to work as expected:
* The updated code has been reviewed.
* Unit tests pass.
* Adding, editing, saving and deleting menus works as expected (including associated errors).

Note You need to log in before you can comment on or make changes to this bug.