search

devict / hacktoberfest

Our own little local version of hacktoberfest
https://hacktoberfest.devict.org
4 stars 18 forks source link

languages can be filtered in table - fixes issue #25 #32

Closed aaronarduino closed 6 years ago

aaronarduino commented 6 years ago

There is probably some optimization to be done here, but this gets us started.

jcbwlkr commented 6 years ago

Oh, hmm yeah I can see that. Still I think I'd rather have one column because space is limited here. If you really care to only see things tagged "CSS" you can put that in the search to filter on it. Probably good enough?

On Thu, Oct 5, 2017 at 2:29 PM, Aaron notifications@github.com wrote:

@aaronarduino commented on this pull request.

In templates/profile.tmpl https://github.com/devict/hacktoberfest/pull/32#discussion_r143035132:

@@ -67,6 +67,9 @@

Title Repository + Lang-1

@jcbwlkr https://github.com/jcbwlkr I was thinking that one column would not sort correctly if the list was something like: [["Go", "CSS", "HTML"], ["HTML", "Go", "CSS"]] With one column, the first language would sort alphabetically but the second and third languages would not. Having three columns would allow for alphabetically sorting all three languages. Not sure which makes the most sense. Though, I might be making a mountain out of a mole hill lol.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/devict/hacktoberfest/pull/32#discussion_r143035132, or mute the thread https://github.com/notifications/unsubscribe-auth/AB7u_96BOcrv--ivrK04IAbAkQLO8U6eks5spS4dgaJpZM4Pujtv .

aaronarduino commented 6 years ago

I agree search is good enough. One column it is then. 👍

jcbwlkr commented 6 years ago

@aaronarduino sweet, thanks for making those changes. I was looking at this more and I noticed languageFetcher is just a struct with one field. If we aren't going to add more fields then we could just do something like type languageFetcher map[string][]string. It would simplify things like this https://gist.github.com/jcbwlkr/0eebde32745c7d784662caaa2e133566

As for the overall issue it looks good but it takes about 6 seconds to get the list of issues which is up from 1 second on master. How can we make this faster server side? Or make the UX better so it doesn't feel so slow?

aaronarduino commented 6 years ago

I'm thinking we might have to do some optimizing on both the front and back end. The languageFetcher should be using goroutines for sure. Maybe we could also load the languages separate from the issues list? At any rate, I think we should add a spinner to the UI to show the user that the list is being retrieved.

jcbwlkr commented 6 years ago

I am planning to redo the profile page so there are different sections which means the issue list will be hidden until you expand that section. In that case I think I don't care haha

aaronarduino commented 6 years ago

I think I can make this code better. I'll be adding another commit in a day or two.

jcbwlkr commented 6 years ago

I had problems with the goroutine based languagefetcher so I reverted back to the last working state. It's a little slow but that's okay. The original branch is still around for reference. I'm going to merge it in this state for now then we should talk about how to speed it up

aaronarduino commented 6 years ago

Yeah, there might be a deadlock problem. It worked ok on my machine so I was hoping it would work for you as well till I got that figured out. I think I need to change where the wg.Done() calls happen. I'll be taking a look at this tonight.