Refactor UI of the Appstore page

[leonardgable]

User wants to browser and install plugins faster. User feedback mentioned bad experience and confusin filters and search on the appstore page. A new AppStore and more modern page has been designed. Easier and less confusing plugin/webapp search is made with a refactor of the filters and the implementation of a new grid component. This constitute a boiler plate for future development on the appstore. The usage of ag-grid improves the user and developer experience to develop new features on top. The library offers great built-in functionality such as cell renderers columns sorting, filtering, quick search etc...

Fortawesome was installed using package json as documented on fontawesome website. Work in progress loose end further implementation are required.

Resolves: #1675

============================================

• [x] vertical alignment is off between the columns
• [x] mismatching use of blue and grey buttons to indicate selection in All|Installed and categories
• [x] table rows should be much tighter, line spacing is way too big
• [x] category buttons should not be grayed out
• [x] sort does not work on Type and Action columns, either disable or fix
• [x] align Type and Action column headers middle so that they match the rows
• [ ] navigating to plugin configuration via cogwheel does not work if the configuration page has something in its search field, maybe clear the search field
• [x] "Please restart the server after installing, updating or deleting a plugin"
• [x] there are two categories named ---?
• [x] installed plugins like Freeboard appear two times in the list when they show up in search result with All plugins selected

[leonardgable]

I have fixed some issue. and I am running out of idea for this story. Is there anything else that needs to be implemented or improved ? I have the impression that maybe I am missing some behaviour when working offline ? Should the appstore be loaded when offline ? Could you please have a look at removing installing and updating plugins making sure everything works as expected ?

[tkurki]

The new design is missing a way to see just "What Updates are available" that we had. If there are updates available we could show a third button [All] [Installed] [Update available].

Idea: we could also have a one click method to "update all installed plugins that have updates available".

[tkurki]

The green small number for available updates on the page looks out of place and unclear what it means

[tkurki]

Some pics of the current state. Some responsiveness related problems:

• [ ] Action and Type are hidden and columns clipped
• [ ] whitespace added if you stretch the windows horizontally

[mairas]

Looking at Teppo's screenshots, good stuff! And Teppo is making good comments, too. :-)

I wonder about the category filters. The pill list looks excessive because there are so many categories and it spans multiple rows already. What if there was, say, a more compact pill list of just the enabled filters, and if you clicked on them, you'd have for example an offcanvas selector slide in for altering the filter list?

[tkurki]

There is value in having all the categories visible and discoverable and not hidden or collapsed.

[santinoo1919]

@mairas thanks for feedback. I think pills number is not too much for a flat structure and > discoverability. Maybe let's ship and improve over time.

[mairas]

Fair enough!

[leonardgable]

Regarding the responsiveness, it works but not on window resize. I will need to find a way to have the grid resize on window resize. Knowing that this is not a very easy task because each column must then have a minimum width from which the column will just disappear and a maximum width from which the column should stop expanding. However not impossible to implement it. I will try to find a nice a neat way to do it. The columns of the a correctly dimensioned if you refresh the page.

Regarding the "Update availables" filter I had the impression there were discussing to remove this button as it wouldn't make sense ? I didnot really understand which final decision was adopted: "Update all" button or "Updates availables" button. Whatever I don't really mind implement one or the other, but please. let's have a final decision on this one. Same thing for the warning message: "Please restart the server after an updates" I had already three or four people telling me to put a different warning message. So please let have a final choice on this one as well. I do not mind changing it again. It takes me 2 minutes. The Categories "---" should not appear as I have modified the category listing on the server side. Maybe you need to run npm run build:all. They do not appear on my side. If this is a problem we will need to hardcode the available categories on the frontend which is not a good idea to me.

Regarding the green badge indicating the number of updates available. I believe this is the last design version I had from Abdou. Is there a new design that I missed ? Could you please share the lastest design, so I can implement it as everybody is expecting it ? I can adopt the "Card" from reactstrap approach this is not an issue.

Thank you for the reviews and I will work on it to meet expectations better ! :+1:

[tkurki]

Regarding the responsiveness, it works but not on window resize. I will need to find a way to have the grid resize on window resize. Knowing that this is not a very easy task because each column must then have a minimum width from which the column will just disappear and a maximum width from which the column should stop expanding.

Everything else in the Admin UI is using the Bootstrap responsiveness framework (and reacting to grid resizes). We have not been very thorough with it, could be improved, but still: why are we not using it here? It provides a way to specify responsiveness without going to the min/max details. Is it because of the table component in use here? What do we gain from using it?

https://react-bootstrap.netlify.app/docs/layout/grid/#responsive-grids

Regarding the "Update availables" filter I had the impression there were discussing to remove this button as it wouldn't make sense ? I didnot really understand which final decision was adopted: "Update all" button or "Updates availables" button.

Imho these are both useful and different things: "show me all updates available for the plugins that I have" and then you can decide if you want to update some of them or for convenience have the "update all" button. The button is less important.

Same thing for the warning message: "Please restart the server after an updates" I had already three or four people telling me to put a different warning message.

Welcome to open source development: people give you comments, opinions and input all over the place. First ideas sometimes don't cover all the use cases - I don't think anyone else was thinking about deleting plugins.

The Categories "---" ...modified the category listing on the server side.

👍 - my bad

Regarding the green badge indicating the number of updates available. I believe this is the last design version...

Not sure if I ever saw a design version with the green either.

In the end when it comes to implementing things we should avoid confusion by treating the Pull Request and its associated comments "the master" - sure, we can and sometimes should discuss things in Discord as discussing there is more convenient, but in the end we need to nail down the implementation and that is in the PR. Goes also for @santinoo1919 - as the design evolves please post updates here, if you create them.

Thank you for the reviews and I will work on it to meet expectations better !

You have so far exceeded my expectations! Discussing PR content, requesting changes and working together is absolutely normal and expected. I have myself created PRs that when I review them myself later on decide that the idea or the approach is not good enough and sometimes need to start from the beginning. More eyes on the matter

[tkurki]

@santinoo1919 @leonardgable are you working together on this? Or are you @leonardgable waiting for my comments?

I created a bunch of todo items in my previous comment, some are not addressed yet. Are those in your work queue @leonardgable ?

[leonardgable]

@santinoo1919 @leonardgable are you working together on this? Or are you @leonardgable waiting for my comments?

I created a bunch of todo items in my previous comment, some are not addressed yet. Are those in your work queue @leonardgable ?

Hi,

I have been super busy lately because I just got a new job and I need to relocate in less than 3 weeks :S

I have addressed most of your point but I would like to organize a catch up with @santinoo1919 to finalize it and be able to ship it. I have corrected responsiveness and already pushed my changes a while ago

[tkurki]

Please help by checking my earlier checklists when you have the time. Not sure if you can put checkmarks there?

[leonardgable]

I cannot check the checkboxes that you have added. It does not work so good

[leonardgable]

After an install or a remove a plugin/app websocket receives updated data which triggers a refresh on the AppStore and also a refresh on the grid's data. Confusing screens refresh to the user occurs. Would it be possible to revise the logic behind the websocket data stream regarding the installation,removal,update of plugins/apps ?

[tkurki]

The websocket event logic's purpose is to support App store, so yes, we can change it.

[tkurki]

I tried a hack: I copied the checklist to the PR's description, where it should be editable by you, check when you have the time. Not sure if this makes sense, it would be great if GH PR review tooling provided this kind of checklists, that are not directly related to specific code lines.

[leonardgable]

The websocket event logic's purpose is to support App store, so yes, we can change it.

I will work on this next in order to improve this. Because it requires so backend changes I will prefer doing this on a second pull request.

Maybe people can already review this and test so we can change it to a Pull requst and ship it, if everyone is satisfied with it

[tkurki]

I’ll have a closer look asap.

Are you sure the ws mechanism needs changing? What do you think is the problem there?

[tkurki]

Looking pretty good! Some minor adjustments:

• Now that the Offline/number of updates indicator shows directly on the menu you always see a red OFFLINE indicator first, when we don't have this data. A red warning like that shown falsely is something that needs fixing.

• on my Mac Chrome the placeholder text does not fit in the search box

  

• I think we should wrap on word boundaries, keeping words whole

• Why is there a "vertical stretch" animation effect on the list (first when you load the list, then if you scroll down a long list like when All is selected)?

• There is no clear indication of what the green number means. @santinoo1919 can you think of a way to tell the user what that means? One idea is that there would be a third top level filter [1 Updates] and the number would be green as it is there now

• I think we should add anyway a button that filters just the installed that have updates, as it is confusing when the only plugin that has an update is below the fold in the installed list (see the picture below - where is the one with the update?)

  

• Name column needs min and max widths, see pics. If the readme links were before the version they would not get staggered and look would be a little tidier

  

• it's a bit unintuitive that if you have more than one screenful of rows, you scroll down and press install the list refreshes to the start and the install progress indicator is hidden below the fold. The user goes "what did just happen - where did the one that I wanted to install disappear". Why do we need to rerender the whole list? The current app store keeps the installed ones always available, also on reloads.

• when using the Update all button the plugin that I have installed disappeared from the installed list, only visible with All selected

• Once you have updated all the UI is in a state where Update all and All buttons look exactly the same, but one is grey for being inactive and the other grey for not being selected, you can click one but not the other. Should the buttons be styled differently? Update All could be hidden if there's nothing to update?

  

[leonardgable]

Thank you Teppo for your comments. My pace had slowed down because I was kept very busy these last days. I have updated the code to adjust and follow your suggestions. Now regarding the updates button it seems that you are not satisfied with the current filters and layout. Can we organize a quick call to make sure we are all on the same line ? I am a bit the impression to turn into a circle. I had a call with santino and to him everyting looked fine but Teppo seems to have other expectations. I guess would be nice to have a clear definition of "done" for this pull request. Otherwise I will keep change back and forth.

My impression is: the app store has already been improved and it's UX and is already way better than what it was before. Can't we deliver it as is and if someone wants to really fine-tune it can be done on top ?

The column width is behaving this way for a reason. With Santino we decided to prioritze the description over the name because it is more meaningful to the user. I have corrected the text-wrap. The placeholder in the search bar is now: "Search"

The updates filter seems to be a "must-have" for teppo. Is it the case ? If yes, I will do it.

What are the "must-have" for this pull-request to be accepted ? Please enumerate a clear list and I will do it. Santino and me had different opinions about the updates filter, now I have the impression we are simply turning in circle.

[tkurki]

Thanks for the fixes!

Here's my wish list:

• find a way so that the red OFFLINE message is not shown initially
• please implement Update available filter
• hide Update All button if there's nothing to update (all updates have finished installing)
• fix when using the Update all button the plugin that I had installed disappeared from the installed list, only visible with All selected

[tkurki]

Can't we deliver it as is and if someone wants to really fine-tune it can be done on top ?

If you ask for a review, as you did explicitly in your previous comment ...people can already review this and test..., then that is what I try to provide. I thought you were waiting for feedback, but now it sounds like you thought this was already finished before you asked? I can't know what you two discuss together, until you tell me. It is also perfectly ok to disagree.

I have tried my best to provide clear feedback on this PR. The best solution is not always obvious and emerges when we implement things and try things out. A good example is the offline message flashing - you can't catch that without actually running the code and it was not a problem with the old ui, because the indicator was not visible on initial load.

Reviews that result in updates and changes are really, really normal in open source projects.

Anyway, I feel this is going to be a great improvement to the Appstore ui!

[leonardgable]

Thanks for the fixes!

Here's my wish list:

• find a way so that the red OFFLINE message is not shown initially

The code was built this way... If you look at the initial appstore the Offline was already appearing. I wouldn't touch that otherwise I will have to modify the websocket logic which is out of the scope of the initial story. The websocket has to be modified on a different merge request. I do not believe this is very detrimental to the users as it is already behaving this way on the current version.

• please implement Update available filter

I had implemented an additional button called "Updates" as "Updates available" would not behave very good with responsiveness

• hide Update All button if there's nothing to update (all updates have finished installing)

Done the button disappear. Santino and I prefered to grey it out because making button disapear can be confusing to the user. Anyway, I have changed it and as soon as the user clicks it it will disappear and udpates all plugin with an available update

• fix when using the Update all button the plugin that I had installed disappeared from the installed list, only visible with All selected

Same comment about the wesocket. The logic that is in place currently generates this kind of behaviour. The way plugins are listed and their status updates using the websocket makes them jump from the "installed" array to the "installing" array. Again I would not change the logic on the websocket as I believe it needs to be addressed in a different pull request. I really do not agree making huge changes at once, it's best to advance with small incremental changes. Websocket logic and the way the information is transferred to the frontend needs to be revised to my opinion

[tkurki]

Seeing this error with the latest version

[tkurki]

I don't think that fixing those items requires websocket api changes.

Please fix the remaining Update All bug and I'll see about finishing the remaining items.

[leonardgable]

Seeing this error with the latest version

Alirght, I had just fixed it. Should be working now

[tkurki]

Please note that I've added some fixes and will be adding a few more.

[tkurki]

@leonardgable the build is failing in CI. The failure is related to ag-grid, do you have any idea why?

Can you describe why you introduced ag-grid instead of just using the components that we had previously?

Ag-grid increases the bundle size a lot: its parsed size is 1.93 MB that is whopping 47% of the resulting 4.15 MB bundle.

[leonardgable]

@leonardgable the build is failing in CI. The failure is related to ag-grid, do you have any idea why?

Can you describe why you introduced ag-grid instead of just using the components that we had previously?

Ag-grid increases the bundle size a lot: its parsed size is 1.93 MB that is whopping 47% of the resulting 4.15 MB bundle.

Hi teppo,

I went over your changes thank you for improving the code and everything made sense to me. Regarding ag-grid is a very powerful javascript library that sincerely boost possibilities with Grids up the roof. I used it because it simplifies logic a lot and will help us to improve the UI a lot if extra features are needed. It is very well documented and is to my opinion the best Grid you may find in javascript. It also allow to increase development speed a lot as it facilitates the implementation of a lot of features

I understand that the bundles becomes very big because of the library that is behind the grid itself. My idea was to improve the other listing in the other menu using ag-grid to reach a consensus and not to have many different libraries in the project.

Ag-grid is an excellent library that could offer a lot to the user if correctly used. Now if you would like to reduce bundle size we could switch to ESM modules instead of using packages. Using ESM modules will help to reduce bundle size as we could import only the code that we need to use.

I have modified the webpack configuration a bit. I took a look at the error and couldn't really figure it out for now. Please retry with the new webpack configuration in the meantime I will investigate more on the error to make it work.

Let me know how the new run works

[leonardgable]

I double checked ag-grid-react code and it seems to me that it only support React 17 and React 18. Knowing that we are using React 16 it could lead to some issue.

Is there any reason we are still using react 16 ? Can we consider upgrading to react 17 ? I can do it if needed

Updating to React 17 would probably solve the issue a lot of fixes and changes were made on the react-dom

[tkurki]

I started looking into using just plain Reactstrap Table for the apps listing, one thing lead to another and I just pushed a version that replaces ag-grid with much simpler code: it now uses just useState, all the useCallback useEffect useMemo and useRef are not needed. So I don't quite see how ag-grid simplifies logic a lot, when all we need is a pretty simple filtered list.

All in all the Admin ui is not very complicated. We can introduce more complex components when there is clear need for them, not for potential future use. Using ag-grid introduced distracting animations, some visual noise (border around a clicked cell) and blew up the bundle size while providing nice mechanism for column responsiveness.

Your point about consensus and not to have many different libraries does not hold, as without a complete rewrite Reactstrap is not going to be removed: any new library will be an addition, nothing will get removed.

Switching to ESM modules and updating React are separate topics, both are possible but not pressing issues. I don't want this PR to snowball.

I believe I have now addressed the earlier issues that I listed:

• text OFFLINE showing - as you see no changes to WebSocket were needed, a really simple fix. The code was not "built this way": it started showing because the menu structure changed
• app disappearing from installed list when updated - again, not related to WebSocket at all

I have also removed some dead code that was not used / was commented out.

The list is not reacting to window resize, but on retrospect the main point of responsiveness is that the UI is usable on mobile device.

All in all this is in mergeable shape now, unless you see something that we should fix.

[leonardgable]

I started looking into using just plain Reactstrap Table for the apps listing, one thing lead to another and I just pushed a version that replaces ag-grid with much simpler code: it now uses just useState, all the useCallback useEffect useMemo and useRef are not needed. So I don't quite see how ag-grid simplifies logic a lot, when all we need is a pretty simple filtered list.

Alright. I am pretty familiar with ag-grid which offers a lot of features, I agree this may be an overkill. Great anyway that you achieved the goal you wanted using reactstrap. I hope my contribution helped anyway even though you had to change a lot of things.. I'll try to be more efficient for the next issue, and work in a more simplistic way All in all the Admin ui is not very complicated. We can introduce more complex components when there is clear need for them, not for potential future use. Using ag-grid introduced distracting animations, some visual noise (border around a clicked cell) and blew up the bundle size while providing nice mechanism for column responsiveness.

Your point about consensus and not to have many different libraries does not hold, as without a complete rewrite Reactstrap is not going to be removed: any new library will be an addition, nothing will get removed.

Switching to ESM modules and updating React are separate topics, both are possible but not pressing issues. I don't want this PR to snowball.

I believe I have now addressed the earlier issues that I listed:

• text OFFLINE showing - as you see no changes to WebSocket were needed, a really simple fix. The code was not "built this way": it started showing because the menu structure changed Yeah I have noticed the fix you did. Well the menu structure changed but anyway it was already showing the offline before hand. But good job, that it doesn't appear on the first page load
• app disappearing from installed list when updated - again, not related to WebSocket at all Perfect, my bad I have some hard time understanding the logic behind and communication between backend and frontend regarding this app store. It is not entirely clear to me

I have also removed some dead code that was not used / was commented out. Great thank you very much

The list is not reacting to window resize, but on retrospect the main point of responsiveness is that the UI is usable on mobile device. I totally agree, to my opinion as I said at the beginning on window resize it is some extra work for responsiveness which is not really needed as the user could refresh the page or simply open the page with the screen size he is most commonly using

All in all this is in mergeable shape now, unless you see something that we should fix. Very nice work, thank you very much Teppor for taking a look at the code and drastically improving it !!!

Thank you very much teppo for your contribution and I'll be happy to see this merge request approved !!

[tkurki]

I hope my contribution helped anyway

You were instrumental in getting this forward! Thank you!