Relations app #50

lustoykov · 2017-03-30T22:18:47Z

#42
after @martomi is done with the API, we can integrate both. @martomi relevant for you is everything in folder /remote

We had requirements that the widgets takes very little space. Try it out on your PC cause the images dont speak too much. Works also if you click suggestion for composer, then it displays pagination with relations.

lustoykov · 2017-03-30T22:19:39Z

react/src/components/relations-app/Application.jsx

+          <Col>
+            <SearchField
+              query={query}
+              className="application__search__field"


rename to application__search-field

lustoykov · 2017-03-30T22:19:56Z

react/src/components/relations-app/Application.jsx

+                  showSuggestions={showSuggestions}
+                  loadingSuggestions={loadingSuggestions}
+                  loadingRelations={loadingRelations}
+                  onRelationSuggestionClick={(_relationSuggestion) => this.onRelationSuggestionClick(_relationSuggestion)}


split lines

lustoykov · 2017-03-30T22:20:00Z

react/src/components/relations-app/Application.jsx

+                  showSuggestions={showSuggestions}
+                  loadingSuggestions={loadingSuggestions}
+                  loadingEntities={loadingEntities}
+                  onEntitySuggestionClick={(_entitySuggestion) => this.onEntitySuggestionClick(_entitySuggestion)}


split lines

vviro · 2017-03-31T10:02:05Z

@lustoykov how about replacing the progress bar with a spinner (or just leaving the space blank while loading). Does it take long to load the results?

Also, would it be possible to test this with the real data provided by the other groups? If it's not possible yet, when do you think we would be able to do that?

lustoykov · 2017-03-31T10:15:55Z

@vviro yes, suggestion taken - will replace with spinner and then you can feedback again. It shouldn't take long to load anyways, so I might leave it empty.

@martomi will provide the API very soon I think (my best bet is latest this weekend). The other groups are working on the database, so it's ultimately up to them to populate the API's database. @martomi do you know how the other groups are progressing regarding DB? I lost the overview there.

martomi · 2017-03-31T10:28:16Z

@lustoykov It's currently a bit chaotic with the API, since there was a major refactor last days and there are still errors that need to be fixed and changes merged before I can do any substantial progress. Also the database is currently empty, I just spoke with other teams, the plan is to populate some test data in the database, but that might take a while since the data needs to be processed by all teams 1-4 before it's ready. My very optimistic guess is today evening for the database, probably tomorrow.

chaoran-chen

Well done, @lustoykov! Your code is clean as always, and I like the concept of the search very much. This is a good beginning for certain!

I'm just not sure if the current presentation of the autosuggestion is really intuitive. I needed quite some moments to realize that those strings under "Suggestions..." are clickable and thought that the search is already finished. I guess that it would better to split the place for the results and the place for the suggestions like Google does with a box right under the search field.

Furthermore, do you think that the subject should be fixed? Because I believe that on a page for Mozart, all kinds of relations relating Mozart should be displayed. That also contains relations where Mozart's father is the subject or where Mozart is an object.

Should I approve it by now, or are you going to implement the suggestion of @vviro right now so that I should wait?

chaoran-chen · 2017-03-31T15:11:32Z

react/src/components/relations-app/remote/loadRelations.js

@@ -0,0 +1,15 @@
+const relations = new Array(100).fill(undefined).map((_, index) => {


Because you've done it at several places, a small info: new Array(n) fills an Array per default with undefined and there is no need to call fill().

And another small info for the unlikely case that you don't know it: you can return a newly created object in an arrow function without return by using (). That means, that you could have written:

.map((_, index) => ({ title: `Relation${index}` }));

lustoykov · 2017-03-31T15:54:46Z

@chaoran-chen thanks, I agree with your suggestions, so don't approve yet. I need to push fixup containing:

Adjust to final API's design
Implement @vviro's suggestions
Implement more intuitive UI (this is challenging me a lot, because we don't have much space. Maybe I should move them under the search bar as suggested?)

Furthermore, do you think that the subject should be fixed? Because I believe that on a page for Mozart, all kinds of relations relating Mozart should be displayed.

dunno how to design that to be honest, we only have as much space as 500 width and 300 height. I think it's ok for now to fix the subject. Let's try to finish this widget so that it works at least for a fixed subject and take it from there, because most of our widgets have begun very ambitiously (the pagination, the graph) and currently neither any of them are in presentable state, nor we actually need them. I'd like to follow the KISS Principle here.

martomi · 2017-03-31T16:08:23Z

I agree with @chaoran-chen that it's not very intuitive. Will definitely require some experimentation to get it right. @lustoykov we hove more than 500 width by the way. That depends on the width of the browser window of course, but most users would have at least 1100px. However if you decide to take advantage of it, you'd need to be able to scale back to less.

lustoykov · 2017-03-31T16:10:35Z

I'll think about it and make some proposals / mockups then so we can vote before implementing it. I'm open to any suggestions, so you can post your ideas as well.

vviro · 2017-04-01T07:48:37Z

Guys, wait, I just realized you are working on the search functionality in the context of a "preview" widget. Therefore not enough space. I'm actually not sure that the search functionality would nicely fit the widget. As @lustoykov mentioned, this is indeed difficult to accomplish. Maybe it can be done, but in any case we cannot use the whole widget for search. Pre-defined entries will also take space, so even if search would be available straight in the widget (which might be a very good idea), it would at first occupy only a small space (for the search bar) and expand only when the user does something with it. Ideally we would have a full-page version of this search view that would be linked from the widget and the widget would only present the top results as an invitation to dive in deeper following the link to the full version. These top results would still take all the widget's space, so the other widget content will be hidden/replaced by it. When the search is cleared or in any other way "closed" or minimized, the widget should return to the initial state. What do you think?

lustoykov · 2017-04-01T16:44:03Z

@vviro what do you mean by predefined entries? We have data in form Entity | Relation | Entity. When we are on a certain page we discussed that one Entity is fixed, so we have Mozart | Relation | Entity. Now we need search bar, when user types in we show suggestions for Relation / Entity (picture 2 and 3). When he clicks Relation or Entity, the widget displays information (picture 3 and 4). So we can't avoid search. As I tried to replicate as much as possible what we've discussed, after this post now I'm even more confused :(

martomi · 2017-04-01T16:48:51Z

I think he means entities that you would expect most users to be interested in. So you show those as pre-defined options instead of requiring a search. If there's not enough space for a search function inside the widget, we can simply show some pre-defined entities (pre-defined search queries if you will) that will show some interesting data. Users can go to the full page to search and explore the rest of the available data we have.

lustoykov · 2017-04-01T16:50:48Z

Thanks for clarifying, makes sense.

Go to mozart
Show predefined entries (relations and entities)
Click on predefined entry (relation or entity)
Show table with Entity | Relation(s) | Entity(ies)

Should we go for this solution, without search? I personally like it better.

@vviro?

martomi · 2017-04-01T16:53:45Z

Yes I like it. We would need an API for those predefined entries.

We'll still have the search, just not inside the widget.

chaoran-chen · 2017-04-01T16:54:18Z

Maybe we could build a simple search (like in #48) in the preview widget and use the complex search in the "expert" mode

lustoykov · 2017-04-01T16:58:15Z

Yees, this might change the API, @martomi take it into consideration.

I'll come up with mockups by tonight. We might as well have modes for the widget, where you can switch between search and predefined entities to save space - I think that's the solution I'm going for. For example in the beginning we have predefined entities and then you click "explore more" they disappear and a search bar appears.

@martomi, @chaoran-chen, @vviro

lustoykov · 2017-04-02T10:10:38Z

Here is the first proposal... a bit difficult to read. Feedback? I will refine more and post another one after the feeedback. Once we agree on the general idea, I will draw a more detailed mockup. Once we agree on a more detailed mockup, I will implement it.

lustoykov · 2017-04-02T23:51:01Z

Soo... Feedback? Anyone? If not, I'll implement it like this and won't accept any complains afterwards :)

kordianbruck · 2017-04-03T00:25:19Z

@lustoykov I mean it looks interesting but without much of an explanation of the parts its hard to follow.

Questions for mode 1: how do you determine what to show in the "explore" section? Do you Mix Entities / Relations?

Mode 2: would basically be the current functionality of this PR?

lustoykov · 2017-04-03T05:44:01Z

@kordianbruck, @vviro was keen on showing predefined entries in the "explore" section - those should be delivered by the backend based on their algorithm. So we thought there would be no place for search bar and we should only go with predefined entries. Then we came up with the idea of two modes - the first mode represents predefined entries and the second is the current PR, yes.

vviro · 2017-04-03T06:43:05Z

@lustoykov Hiding the search bar behind a mode switch will probably prevent 90% of users from using it or learning about its existence in the first place. So if the search bar makes any sense in a given context, it should be visible from the beginning.

We definitely want to have pre-defined relations and related entities displayed from the beginning (when the user sees the widget/page for the first time). The question is how to structure them. Is this a bag of tags? A list of hand-picked sections with a few entries in every section? How many such entries do we plan to display? What is the maximum number? What are the implications for the API? How does the interaction with those items work? Is mouse-over involved? What happens when one clicks on a relation or entity? Can one click on the whole section and see more relations/entities of the same kind? If so, does one leave this page and go somewhere else, or how does the view change? I think it would be very useful to plan these details through before starting with the implementation.

kordianbruck · 2017-04-03T10:10:54Z

@vviro that is a lot of questions that I'm not sure this is the right place to discuss them. We should have a meeting for that over hangouts or in person in order to clarify this.

Regarding this PR: lets finish it up, move the open stuff to an issue so we can merge an continue. This thread is getting kinda long.

I like the idea of having to predefined terms - lets just move it to an issue and deal with them there.

lustoykov · 2017-04-03T11:15:02Z

ok, team, can we merge it as it is and improve further in a separate thread as suggested by @kordianbruck?
@chaoran-chen, @martomi, @Binabiko?

I think changes don't makes sense at this point without getting the requirements right in the first place.

What's your opinion?

martomi

I agree. Let's merge this version and will iterate on it with subsequent PRs.

joern-vh · 2017-04-03T12:04:37Z

I agree, too.

martomi · 2017-04-03T12:05:34Z

Maybe look at the codacy issues before merging.

chaoran-chen · 2017-04-03T13:09:22Z

I think that the code style is good as it is. We have our own .eslint config and it is enough to fulfill that.

kordianbruck · 2017-04-03T13:21:35Z

@chaoran-chen it is using your configuration file. But you can really ignore it if you fine with the results.

chaoran-chen · 2017-04-03T13:30:49Z

Oh, that's strange because, in our setup, we are not getting those issues. Does codacy also consider the eslint-configuration versions that are defined in package.json?

lustoykov · 2017-04-03T14:40:42Z

Our ling config / modules are a bit outdated and doesn't have the same version as codacy, thus the difference.

lustoykov added 7 commits March 31, 2017 00:14

React: Add Relations entry point

dd656bf

React: Add styles

2617320

React: Mockup relations

677347a

React: Add Application for relations

e103e59

React: Add SearchField for Relations

39c2e7a

React: Add Entity components for Relations

f6bf335

React: Add Relation components for Relatiosn

d5c1d1a

lustoykov self-assigned this Mar 30, 2017

lustoykov requested review from martomi, chaoran-chen and joern-vh March 30, 2017 22:18

lustoykov commented Mar 30, 2017

View reviewed changes

kordianbruck mentioned this pull request Mar 31, 2017

Implement Pagination MusicConnectionMachine/VisualizationG6#49

Closed

chaoran-chen reviewed Mar 31, 2017

View reviewed changes

kordianbruck mentioned this pull request Apr 2, 2017

Integrate Relations in Search Results MusicConnectionMachine/VisualizationG6#51

Closed

martomi approved these changes Apr 3, 2017

View reviewed changes

chaoran-chen approved these changes Apr 3, 2017

View reviewed changes

lustoykov merged commit 4468c3a into develop Apr 3, 2017

chaoran-chen deleted the relations-app branch April 5, 2017 20:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relations app #50

Relations app #50

lustoykov commented Mar 30, 2017 •

edited

Loading

lustoykov Mar 30, 2017

lustoykov Mar 30, 2017

lustoykov Mar 30, 2017

vviro commented Mar 31, 2017

lustoykov commented Mar 31, 2017 •

edited

Loading

martomi commented Mar 31, 2017 •

edited

Loading

chaoran-chen left a comment •

edited

Loading

chaoran-chen Mar 31, 2017 •

edited

Loading

lustoykov commented Mar 31, 2017 •

edited

Loading

martomi commented Mar 31, 2017 •

edited

Loading

lustoykov commented Mar 31, 2017 •

edited

Loading

vviro commented Apr 1, 2017

lustoykov commented Apr 1, 2017

martomi commented Apr 1, 2017

lustoykov commented Apr 1, 2017 •

edited

Loading

martomi commented Apr 1, 2017

chaoran-chen commented Apr 1, 2017

lustoykov commented Apr 1, 2017 •

edited

Loading

lustoykov commented Apr 2, 2017

lustoykov commented Apr 2, 2017

kordianbruck commented Apr 3, 2017

lustoykov commented Apr 3, 2017

vviro commented Apr 3, 2017

kordianbruck commented Apr 3, 2017

lustoykov commented Apr 3, 2017 •

edited

Loading

martomi left a comment

joern-vh commented Apr 3, 2017

martomi commented Apr 3, 2017

chaoran-chen commented Apr 3, 2017

kordianbruck commented Apr 3, 2017

chaoran-chen commented Apr 3, 2017

lustoykov commented Apr 3, 2017 •

edited

Loading

		@@ -0,0 +1,15 @@
		const relations = new Array(100).fill(undefined).map((_, index) => {

Relations app #50

Relations app #50

Conversation

lustoykov commented Mar 30, 2017 • edited Loading

lustoykov Mar 30, 2017

Choose a reason for hiding this comment

lustoykov Mar 30, 2017

Choose a reason for hiding this comment

lustoykov Mar 30, 2017

Choose a reason for hiding this comment

vviro commented Mar 31, 2017

lustoykov commented Mar 31, 2017 • edited Loading

martomi commented Mar 31, 2017 • edited Loading

chaoran-chen left a comment • edited Loading

Choose a reason for hiding this comment

chaoran-chen Mar 31, 2017 • edited Loading

Choose a reason for hiding this comment

lustoykov commented Mar 31, 2017 • edited Loading

martomi commented Mar 31, 2017 • edited Loading

lustoykov commented Mar 31, 2017 • edited Loading

vviro commented Apr 1, 2017

lustoykov commented Apr 1, 2017

martomi commented Apr 1, 2017

lustoykov commented Apr 1, 2017 • edited Loading

martomi commented Apr 1, 2017

chaoran-chen commented Apr 1, 2017

lustoykov commented Apr 1, 2017 • edited Loading

lustoykov commented Apr 2, 2017

lustoykov commented Apr 2, 2017

kordianbruck commented Apr 3, 2017

lustoykov commented Apr 3, 2017

vviro commented Apr 3, 2017

kordianbruck commented Apr 3, 2017

lustoykov commented Apr 3, 2017 • edited Loading

martomi left a comment

Choose a reason for hiding this comment

joern-vh commented Apr 3, 2017

martomi commented Apr 3, 2017

chaoran-chen commented Apr 3, 2017

kordianbruck commented Apr 3, 2017

chaoran-chen commented Apr 3, 2017

lustoykov commented Apr 3, 2017 • edited Loading

lustoykov commented Mar 30, 2017 •

edited

Loading

lustoykov commented Mar 31, 2017 •

edited

Loading

martomi commented Mar 31, 2017 •

edited

Loading

chaoran-chen left a comment •

edited

Loading

chaoran-chen Mar 31, 2017 •

edited

Loading

lustoykov commented Mar 31, 2017 •

edited

Loading

martomi commented Mar 31, 2017 •

edited

Loading

lustoykov commented Mar 31, 2017 •

edited

Loading

lustoykov commented Apr 1, 2017 •

edited

Loading

lustoykov commented Apr 1, 2017 •

edited

Loading

lustoykov commented Apr 3, 2017 •

edited

Loading

lustoykov commented Apr 3, 2017 •

edited

Loading