DMPRoadmap / roadmap

DCC/UC3 collaboration for a data management planning tool
MIT License
102 stars 109 forks source link

Standardize tables with more than 10 items #815

Closed stephaniesimms closed 6 years ago

stephaniesimms commented 6 years ago

Related #786 Implement pagination and add a search box above any table within the system that exceeds 10 items

Add pagination logic to the following pages:

Search box example screen shot 2017-11-13 at 3 29 47 pm

Pagination example screen shot 2017-11-13 at 3 30 18 pm

briri commented 6 years ago

Change the existing table search box so that it looks like the search box above (change placeholder text and position). Also make sure that all of these tables has have sortable columns (where applicable).

briri commented 6 years ago

Using the 'TABLE_FILTER_MIN_ROWS' for both the size of the paginated table and the trigger for whether or not the search/filter box appears is problematic. Paginated tables now only ever show 10 rows so the search/filter box is always hidden.

Would probably be better to trigger the display of the search/filter box via JS and the presence of more than one 1 page.

jollopre commented 6 years ago

I thought we would display that search box only when View all is clicked, i.e. when all the records for a model are shown, otherwise it is not displayed.

stephaniesimms commented 6 years ago

the search box should be displayed if there are more than 10 items in a table - regardless of whether the user clicks to view all or not. it should still be possible to search the long table without having to expand it to see the full contents.

jollopre commented 6 years ago

@stephaniesimms if that's the case we will need to create MORE controller actions, one for each model (e.g. plan, template, user, etc) that we want to search for data according to any field.

briri commented 6 years ago

@jollopre in the DMPTool code we are passing the search/filter value in as a param . We should be able to do something similar. We could then let the pagination handle however many results are returned by the search criteria. You can see an example of it in use on the public Templates page: https://dmptool.org/guidance Here's is the controller logic for that page: https://github.com/CDLUC3/dmptool/blob/7650b5e551c3ab640a97d92eb4d627f88078243e/app/controllers/static_pages_controller.rb#L103 and the model logic that does the actual search: https://github.com/CDLUC3/dmptool/blob/7650b5e551c3ab640a97d92eb4d627f88078243e/app/models/requirements_template.rb#L79

briri commented 6 years ago

after doing some testing I noticed that the current column sorts are behaving in the same manner so we would need to follow the same pattern as mentioned above for that as well.

An alternative would be to scrap the use of the pagination gem and find a suitable JS alternative that would complement our existing filter/sort/search. In my opinion the JS approach would be preferable since it reduces the number of server calls. With that said though, when tables get large (e.g. for a super admin) the load time of the page increases due to the fact that we're not doing the pagination server side. I'm not sure if their's a good pluggable JS library to help us.

jollopre commented 6 years ago

Moving towards a JS approach of retrieving all at once can increase significantly the burden on the server when the data to be retrieved is big, for instance (templates or organisational plans). It is considered a bad practice to retrieve more things that the user really needs too.

In my opinion, it is better to treat functionalities such as search as separated actions in controllers which are called through AJAX. In contrast, the sorting action can be done directly on JS since you already got the data loaded.

dsisu commented 6 years ago

It's important you all agree. It sounds as if you need to discuss this issue together before you move on to implementing any quick fixes that will then require a lot of work to maintain. Please arrange a skype call if you need to move on with this before your usual developers' meeting on Friday.

briri commented 6 years ago

agreed @dsisu! I'll add it to the dev discussion for Friday. I'm going to move this ticket down in priority. I confirmed with @stephaniesimms that its of less importance for the Monday template editing party. We need to come up with a single solution and make sure its being used across the tool.

sjDCC commented 6 years ago

@jollopre I raised a couple of issues in #991 but I think they should be covered by this.

Ideally we'd let people search and reorder the full content at any time, but if that has detrimental impacts in terms of performance then perhaps we need to reconsider?

The way @briri is proposing to separate out the tabs for org-admin and super-admin (see #900) means that the only huge view would be the 'All templates' one for super-admins.

jollopre commented 6 years ago

the latest PR includes a generic searchable functionality for:

templates search will be handled by #1026 and the sorting mechanism by #1027. Please, can @sjDCC and @stephaniesimms could you test the new UI behaviour?

sjDCC commented 6 years ago

Search looks good, thanks @jollopre

Only suggestion I have is to add a 'View all' button that lets users view all search results if they go over 1 page e.g. "View all search results". Currently clicking this return you to all data, rather than just all search results. If the users has entered a broad search term like below, results may be multiple pages of the table. I blocked out names and emails for sensitivity reasons.

users

Leaving this open for @stephaniesimms to test and comment but happy to close out

stephaniesimms commented 6 years ago

I created a new issue #1055 to capture all pages where the search box should be implemented, per our call today.

I noticed that it appears on My Dashboard for the Org plans table that only contains 1 plan (screenshot below). The same logic should apply for pagination and displaying the search box - that is both should only be visible if there are more than 10 items in the table. Please remove them from the Org plans table @jollopre

screen shot 2018-01-16 at 7 48 20 pm

jollopre commented 6 years ago

@stephaniesimms, PR #1057 includes the logic to hide the search functionality if the results to be presented in a table a <= 10.

stephaniesimms commented 6 years ago

all working as expected now. closing issue.