[$130] Cant Found new Group After Succesfully Created

[citra-rahman]

Step To Reproduce

1. Go to https://skill-search.topcoder-dev.com/
2. Login as ToniJ(topcoder)
3. Go to middle tab (create group tab)
4. Type javascript in search box
5. Click create button
6. search javascript in the same search box

Expected Result

We can search the new group after successfully added it.

Actual Result

Cant found new group after successfully created

Screenshot

Video

https://www.dropbox.com/s/t9jvrg161pr0f59/U-Bahn%20employee%20skills%20search%20portal%20-%20Google%20Chrome%202020-07-12%2017-02-45.mp4?dl=0

Device/OS/Browser Information:

a. Device: Lenovo (laptop)
Processor : AMD A9-9420 RADEON R5, 5 Compute Cores 2C+3G 3.00 GHz
RAM : 8 GB
System Type: 64 Bit OS, x64-based processor
Pen or touch: no pen or touch
b. OS: Windows 10 Home Single Language
c. Browser: Chrome Version 83.0.4103.106 (Official Build) (64-bit)

[callmekatootie]

Groups are paginated. We seem to be only showing first n values but in reality, we need to paginate them. Probably a design issue? 🤔

[callmekatootie]

@wdprice How should we handle this - both in the group tab and in the add group modal - should we fetch all the groups in one stretch or should we paginate - if the latter, then should we perhaps first do a design update?

[wdprice]

@callmekatootie trying to better understand - this issue only takes place when we do a search, correct?

[callmekatootie]

Yes and No.

So right now, when we display the groups (both in the modal as well as in the Groups Tab page), we are displaying only the first n groups. There's more groups to be displayed but we don't do that right now - the results are paginated.

So, at the time of search as well as at the time of displaying all the groups, we need to determine how do we show them. Right now, iirc, the number of groups that we have in dev environment is upwards of 150

[wdprice]

We should plan for a lot of groups - add pagination in the UI for scenarios in which more than n groups are returned.

[callmekatootie]

Expected:

For both the groups tab and the Add to Group modal

• When we fetch the groups, set a large value for items returned per page - to get ALL the groups
• Use virtual scroll. In case of the groups tab, restrict the number of groups shown to the current height (where we are displaying about 12 groups. In case of the modal, we already have a scrollbar when we display too many groups. Reuse the same because the height of the modal fits the screen it is in and we do not want to lose this functionality
• When the user searches, we will search among all the groups that we have just as we do currently. No changes expected here.
• Read the value for the items per page from environment variable REACT_APP_GROUPS_PER_PAGE. Let it have a default value of 1000 for now.

[callmekatootie]

Ticket is now open for pickup. PLEASE make sure that you read the requirements here - #155 (comment)

[cwdcwd]

Contest https://www.topcoder.com/challenges/30134488 has been created for this ticket.

This is an automated message for lazybaer via Topcoder X

[cwdcwd]

Contest https://www.topcoder.com/challenges/30134488 has been updated - it has been assigned to will_price.

This is an automated message for lazybaer via Topcoder X

[cagdas001]

@callmekatootie pushed changes at #619

[callmekatootie]

@cagdas001 Correct me if I am wrong - the changes seem to be only for the side menu. I could not see the changes carried out in the Group modal

[cagdas001]

@callmekatootie You mean AddToGroup modal right?

It already has a scrollable content as you also said

In case of the modal, we already have a scrollbar when we display too many groups. Reuse the same because the height of the modal fits the screen it is in and we do not want to lose this functionality

[callmekatootie]

We have scrollable content but no virtual lists - you need to use virtual lists here, while respecting the existing methodology of setting the height of the modal

[cagdas001]

I thought the qouted text meant to re-use the same logic. Ok, will replace it and push a new commit soon

[cagdas001]

@callmekatootie I have updated the PR. Scrollbars are currently native browser scrollbars. I have not added any styling for them, since it's not possible to do it with cross-browser support with pure CSS. If we'd have a bit styling for scrollbars it might look better, however, we need to use a JavaScript scrollbar like https://github.com/Grsmto/simplebar to achieve this

[callmekatootie]

Yep. That is ok. Thank you. Will be reviewing shortly

[callmekatootie]

@cagdas001 Excellent stuff. There's one minor issue that I came across. Please see below gif:

There are unidentical gaps between two rows.

I did not find this issue in the group modal - methinks it is because the items there are always on a single line, with overflow on ellipsis... and a tooltip... But I could be wrong.

If that is indeed the cause, could you update the group items in the sidebar to have a single row too with overflow on ellipsis and tolltip... Be sure to test with a group name that has no spaces in it...

[cagdas001]

I guess there is a mistake in rowHeight calculation, where it calculates the heights of rows based on ´group.name.length´

Currently it adds 20px for each extra line, and assumes lines are being wrapped to the next line at the 30th character. However looks like 30 is not a certain number, some rows have 33 characters in their first lines while some others have 27, 31, 34...

If we can write a good implementation of rowHeight function that addresses this issue, I think it should work even with multi line rows.

Single line solution with ellipsis would also work

[callmekatootie]

Ok. Let's go with the single line solution. Could you do that for an additional $10? I imagine just the CSS needs to be updated and a tooltip needs to be attached

[cagdas001]

Yes single line solution is easier to apply Just a few CSS rules and some minor changes (to remove dynamic height calculation)

[cagdas001]

@callmekatootie done

[callmekatootie]

@cagdas001 In IE11, there's a horizontal scrollbar that is displayed in the Add Group modal:

otherwise in IE11 the changes are fine

[callmekatootie]

@cagdas001 Have you checked out the above - would like to resolve this asap

[cagdas001]

@callmekatootie yes I'm on this right now. will provide a fix soon

[callmekatootie]

@cagdas001 Hoping I get a solution in the next 1 hour else I am afraid I will have to deduct the payment on this ticket and assign the task of resolving the issue to another member.

[cwdcwd]

Payment task has been updated: https://software.topcoder.com/review/actions/ViewProjectDetails?pid=30134488

This is an automated message for lazybaer via Topcoder X

[cagdas001]

@callmekatootie That's OK, it's not a problem. But this issue is not a simple issue as it looks since browsers are behaving differently when it comes to the content width with scrollbars. When reviewing the solution, I'd suggest you to test the solution not only on IE but all major browsers, to see if it's working consistently, without breaking the existing layout (with unnecessary gaps etc)