MIR-1151 Rework mods name search #776

sebhofmann · 2022-06-16T14:55:21Z

kkrebs

feature works and looks good for me, I only see some little fine tuning to do. See code comments.

kkrebs · 2022-07-18T12:47:48Z

mir-webapp/src/main/vue/name-search/public/index.html

+    <meta http-equiv="X-UA-Compatible" content="IE=edge">
+    <meta name="viewport" content="width=device-width,initial-scale=1.0">
+    <link rel="icon" href="<%= BASE_URL %>favicon.ico">
+    <link rel="stylesheet" href="http://localhost:8291/mir/rsc/sass/mir-layout/scss/flatmir-flatly.min.css">


please check href to localhost

index.html is only used for testing its not actually used when the component is built.

kkrebs · 2022-07-18T12:48:57Z

mir-webapp/src/main/vue/name-search/public/index.html

+      <script>
+          window["webApplicationBaseURL"] = "http://localhost:8291/mir/";
+          window["currentLang"] = "de";
+      </script>


are these default values only?

index.html is only used for testing its not actually used when the component is built.

kkrebs · 2022-07-18T12:57:51Z

mir-webapp/src/main/vue/name-search/src/App.vue

+}
+</script>
+
+<style scoped>


We should use this feature with care and better put css / sass code to mir-layout. This way we can better ensure a consistent look and feel. Please check if all used classes are only relevant for this vue component.

kkrebs · 2022-07-18T12:59:17Z

mir-module/src/main/resources/config/mir/messages_de.properties

+mir.editor.person.website           = Website
+mir.editor.person.link              = Link
+mir.editor.person.wikipedia         = Wikipedia
+mir.editor.addIdentifier            = Identifier Hinzuf\u00FCgen


little type here -> "hinzufügen"

golsch · 2022-07-20T08:10:41Z

mir-module/src/main/resources/config/mir/messages_de.properties

+mir.editor.person.website           = Website
+mir.editor.person.link              = Link
+mir.editor.person.wikipedia         = Wikipedia
+mir.editor.addIdentifier            = Identifier Hinzuf\u00FCgen


Respect to "mir.editor.person.applyPerson", "ID" instead of "Identifier"?

golsch · 2022-07-20T08:18:59Z

pom.xml

@@ -45,7 +45,7 @@
    <jetty.version>11.0.9</jetty.version>
    <maven.compiler.arg>-Werror</maven.compiler.arg>
    <mycore.version>2022.05-SNAPSHOT</mycore.version>
-    <node.version>v12.16.3</node.version>
+    <node.version>v12.22.12</node.version>


Does not belong directly to it, but this props are redefined in MIR, may we should use definitions of mycore-parent, according to MyCoRe-Org/mycore#1623

golsch · 2022-07-20T08:21:10Z

mir-webapp/src/main/vue/name-search/src/App.vue

+            <i class="fas fa-address-card"></i>
+            <span class="identifier-count">{{ currentIdentifier.length }}</span>
+          </button>
+          <button :id="`search-${this.nameIndex}`" type="button" class="btn btn-outline-secondary"


Oher templates use "btn-secondary" here

golsch · 2022-07-20T08:35:34Z

mir-webapp/src/main/vue/name-search/src/App.vue

+  }
+
+  closeDrops() {
+    console.log("Clicked outside!")


I would generally suggest to remove the logs in production.

golsch · 2022-07-20T08:37:33Z

mir-webapp/src/main/vue/name-search/src/api/ProxiedOrcidProvider.ts

+    }
+
+    get url() {
+        return (<any>window)["webApplicationBaseURL"] + "servlets/MIROrcidServlet"


Type any leads to ugly outputs during building. I would suggest to annotate them at least so that they are no longer displayed

golsch · 2022-07-20T08:38:13Z

mir-webapp/src/main/vue/name-search/src/api/LocalSearchProvider.ts

+
+        for (let i = 0; i < array.length; i+=2) {
+            const term = array[i] as string;
+            const count = array[i+1] as number;


- use vue.js for the GUI - allow multiple identifiers to be added with a search - more detailed result display - added ror to search for corporate

* fixed typos * removed log messages * replaced <any>window with window as any * replace btn-outline with btn-secondary * use node and yarn version of mycore-parent * remove unused variable

* 2022.06.x: MCR-1116 added rules for new fact based acl system MCR-2728 remove jpa mapping from mycore-ifs MCR-2530 add translations MCR-2731 Add configurable list of x-languages that may be edited using the classification editor (#797) MIR-1151 Rework mods name search (#776) MIR-1173 use only relatedItems with right type and id (#806) MIR-1170 Provide default value to prevent unintentional behavioural change Bump moment from 2.29.2 to 2.29.4 in /mir-module Fix bibutils download link

sebhofmann force-pushed the issues/MIR-1151-Rework_mods_name_search branch from 7df2f15 to 45c5f60 Compare June 17, 2022 12:04

kkrebs requested review from golsch and toKrause June 21, 2022 13:44

kkrebs requested changes Jul 18, 2022

View reviewed changes

golsch requested changes Jul 20, 2022

View reviewed changes

sebhofmann requested a review from golsch July 27, 2022 14:25

golsch approved these changes Jul 28, 2022

View reviewed changes

sebhofmann changed the base branch from main to 2022.06.x August 16, 2022 12:32

sebhofmann force-pushed the issues/MIR-1151-Rework_mods_name_search branch from 5131781 to 4990d8f Compare August 30, 2022 13:36

kkrebs approved these changes Sep 21, 2022

View reviewed changes

sebhofmann added 5 commits September 22, 2022 09:59

MIR-1151 Rework mods name search

6921684

- use vue.js for the GUI - allow multiple identifiers to be added with a search - more detailed result display - added ror to search for corporate

Fix tests

94fa32e

Improve Searching

48abc20

major improvements

300c7a4

MIR-1151 added fixes from review

f274c75

* fixed typos * removed log messages * replaced <any>window with window as any * replace btn-outline with btn-secondary * use node and yarn version of mycore-parent * remove unused variable

sebhofmann force-pushed the issues/MIR-1151-Rework_mods_name_search branch from 4990d8f to f274c75 Compare September 22, 2022 07:59

MIR-1151 fix tests

f394ff7

sebhofmann merged commit a7f278f into 2022.06.x Sep 27, 2022

sebhofmann deleted the issues/MIR-1151-Rework_mods_name_search branch September 27, 2022 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MIR-1151 Rework mods name search #776

MIR-1151 Rework mods name search #776

sebhofmann commented Jun 16, 2022 •

edited by fluetze

Loading

kkrebs left a comment

kkrebs Jul 18, 2022

sebhofmann Jul 19, 2022

kkrebs Jul 18, 2022

sebhofmann Jul 19, 2022

kkrebs Jul 18, 2022

kkrebs Jul 18, 2022

golsch Jul 20, 2022

golsch Jul 20, 2022

golsch Jul 20, 2022

golsch Jul 20, 2022

golsch Jul 20, 2022

golsch Jul 20, 2022

MIR-1151 Rework mods name search #776

MIR-1151 Rework mods name search #776

Conversation

sebhofmann commented Jun 16, 2022 • edited by fluetze Loading

kkrebs left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebhofmann commented Jun 16, 2022 •

edited by fluetze

Loading