add referencePreview feature #140

Kiailandi · 2017-11-17T16:35:00Z

This PR will add a new feature to the PST.

The referencePreview feature will enable the user to preview (and to approve or reject) a reference without opening it in another tab.

If the reference is fetched from a dataset that makes its corpus available (e.g. StrepHit) the corpus itself will be used to create a preview.

If the corpus is not available (e.g. Freebase) a generic scraper will be used to create a "best effort" preview.

In the preview the item label, the property label and the value will be highlighted (yellow marker in the previous image) through text search.

- fix a bug where all urls would be concatenated as a common url - fix a visual bug where the preview button would be cut in case of short urls - refactor the code used to append the preview button into a general function

…its call

sjoerddebruin · 2017-12-01T16:57:20Z

Can the button be more subtile? It's quite prominent now for such a feature.

Kiailandi · 2017-12-01T17:28:03Z

Yes, it can. I will try a different style in the next few days 😃

marfox · 2017-12-04T10:18:52Z

@sjoerddebruin , maybe you can give an example of what you mean with subtile ?

sjoerddebruin · 2017-12-04T10:19:32Z

A black button seems very off with the rest of the Wikidata user interface.

Kiailandi · 2017-12-04T13:56:43Z

I used white as a background and copied the hex of the other elements as color.

Tpt

Thank you very much for this change. Here are some comments related mostly on how to improve the implementation.

PS: There is also a conflict with the current master version. May you do a rebase?

Tpt · 2017-12-04T15:33:40Z

frontend/freebase2wikidata.js


  var async = module.exports;
+  var ps = mw.ps || {};


I am not sure that having a such global is needed. You could just declare the openNav and closeNav functions inside of the main PrimarySources closure and call them.

I initially put the functions in the closure, as you suggested. The problem is that they are called from an onClick of a button, and they are not visible from outside of the closure. If you have any suggestion on how to fix this I could remove the global object.

I see. Instead of building the HTML from a string and on* attributes you could maybe build it using jQuery or something similar, allowing you to call functions declared in the closure. Eg.

$('<div>').attr('id', 'myNav').addClass('overlay') .append( $('<a>').addClass('closebtn).click(closeNav), $('<div>').attr('id', "blackboard").addClass("overlay-content") );

It's probably even going to make more sense if you use OOjs UI.

Tpt · 2017-12-04T15:58:41Z

frontend/freebase2wikidata.js

+            type: 'GET',
+            url: 'https://tools.wmflabs.org/strephit/search?url=' + encodeURI(referenceURL),
+            success: function(msg) {
+                $('.loader').remove();


You could use the OOjs UI progress bar to fit more into MediaWiki UI and to avoid to have to write extra code https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.ProgressBarWidget

The loader is pure css and it is not related to "real" progress, it is just used to signal the user that the tool is not blocked but it is waiting for the backend.

Ok! If you do not set specific progress level OO.ui.ProgressBarWidget is going to be in "indeterminate" state and display a visual motion.

Tpt · 2017-12-04T16:02:26Z

frontend/freebase2wikidata.js

+                            var item = data[index];
+                            if(index != 'url' && item !== null){
+                                if( index == 'other' ) {
+                                    while(typeof item == 'string'){


Seems quite dangerous (but it's maybe what should be done)

I can see your point, but atm I'm not able to provide a better solution for this, as I don't have a consistent format for the data I get from the api.

Tpt · 2017-12-04T16:03:00Z

frontend/freebase2wikidata.js

@@ -34,9 +34,9 @@
 */

 $(function() {
-  'use strict';


Do you have a good reason to drop the "use strict" option?

We dropped it in this commit, if you remember, as a temporary fix for the tool. I also think that this is the source of the conflict as it is a duplicate commit.

Ok, thank you! Sorry, for not remembering.

Tpt · 2017-12-04T16:05:39Z

frontend/freebase2wikidata.js

+                                        }
+                                    }
+                                }
+                                else {


This file seems to use the K&R coding style. Example:

if(foo) { bar; } else { baz; }

It's not important but it's better for consistency.

Will fix this

Tpt · 2017-12-04T16:20:07Z

frontend/freebase2wikidata.js

+    ps.openNav = function openNav(itemLabel, propertyLabel, propertyValue, referenceURL, buttons) {
+        $('#myNav').width('100%');
+
+        var blackboard = $('#blackboard');


Instead of building your own window system you could maybe rely on https://doc.wikimedia.org/oojs-ui/master/js/#!/api/OO.ui.ProcessDialog that seems to fit quite well this use case.

PrimarySources is already using it for the PrimarySources list feature and there is already a windowManager created at the end of this file.

This is a great suggestion. I used a simple div as a window system as it seemed the fastest and easiest way to me. I will try to look into the OO.ui library.

Kiailandi added 5 commits October 6, 2017 10:39

remove use strict as a temporary fix

fe6bebe

add referencePreview module

70da2dc

fix css indentation

2d8be3f

fix minor bugs and refactor code to be more modular

3206b5d

- fix a bug where all urls would be concatenated as a common url - fix a visual bug where the preview button would be cut in case of short urls - refactor the code used to append the preview button into a general function

move the fetch of the buttons from the body of the openNav method to …

794a4aa

…its call

change preview button colors to integrate better with UI

40911cc

Tpt reviewed Dec 4, 2017

View reviewed changes

Kiailandi and others added 2 commits December 4, 2017 18:29

fix if else style

fe67f2e

Merge branch 'master' into master

40a9666

marfox merged commit 40a9666 into Wikidata:master Jan 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add referencePreview feature #140

add referencePreview feature #140

Kiailandi commented Nov 17, 2017

sjoerddebruin commented Dec 1, 2017

Kiailandi commented Dec 1, 2017

marfox commented Dec 4, 2017

sjoerddebruin commented Dec 4, 2017

Kiailandi commented Dec 4, 2017

Tpt left a comment •

edited

Loading

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017

Tpt Dec 4, 2017

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017 •

edited

Loading

Tpt Dec 4, 2017

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017 •

edited

Loading

Tpt Dec 4, 2017

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017

Tpt Dec 4, 2017

Kiailandi Dec 4, 2017

add referencePreview feature #140

add referencePreview feature #140

Conversation

Kiailandi commented Nov 17, 2017

sjoerddebruin commented Dec 1, 2017

Kiailandi commented Dec 1, 2017

marfox commented Dec 4, 2017

sjoerddebruin commented Dec 4, 2017

Kiailandi commented Dec 4, 2017

Tpt left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kiailandi Dec 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kiailandi Dec 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tpt left a comment •

edited

Loading

Kiailandi Dec 4, 2017 •

edited

Loading

Kiailandi Dec 4, 2017 •

edited

Loading