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

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

•  vertical alignment is off between the columns
• mismatching use of blue and grey buttons to indicate selection in All|Installed and categories
• table rows should be much tighter, line spacing is way too big
• category buttons should not be grayed out
•  sort does not work on Type and Action columns, either disable or fix
• 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
• "Please restart the server after installing, updating or deleting a plugin"
•  there are two categories named ---?
•  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]

Please review against this picture:

•  vertical alignment is off between the columns
• mismatching use of blue and grey buttons to indicate selection in All|Installed and categories
• table rows should be much tighter, line spacing is way too big
• category buttons should not be grayed out
•  sort does not work on Type and Action columns, either disable or fix
• 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
• "Please restart the server after installing, updating or deleting a plugin"
•  there are two categories named ---?
•  installed plugins like Freeboard appear two times in the list when they show up in search result with All plugins selected

[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 ! 👍

[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]

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!