Implemented Bookmark Functionality. #195

sakshi-dhamija · 2021-05-16T08:35:18Z

Checklist

I indicated which issue (if any) is closed with this PR using closing keywords
I only changed lines related to my PR (no bulk reformatting)
I indicated the source and check the license if I re-use code, or I did not re-use code
I made my best to solve only one issue in this PR, or explain why multi had to be solved at once.

Issue

fix #91

Details

Features Implemented:

Two-column layout for bigger screens and single-column layout for screens with width<=600px.
Bookmark icon addition near the edit and add icon.
Stores the state of the tree. We can open the bookmarked state on the tree by clicking on the term.
No loss on reloading: Bookmarked terms would not be lost even if the user reloads the page. They will be removed only when a new session is opened.

Shortcomings:

Currently, a user cannot un-bookmark the term or remove the term from bookmarks. Also, by standard practices, the color of the bookmark icon should change when a term is bookmarked. I tried to implement this but faced several issues regarding the session storage.
There is nowhere explicitly stated that this is the bookmarked terms section. I tried different approaches to display this, but none of it aligned with the UI.

Future Scope Of Improvement:

Similar to tooltips, we can add the functionality that details of bookmarked terms will appear when we hover over the names.
Better UI.

Implementing this feature took me a while since I used some concepts which are relatively new to me. If @bryan-brancotte @matuskalas @hmenager or anyone from the contributors has any suggestions to overcome the shortcomings or other improvements or even code-reduction, please let me know.

Updating Repo!

Updating main!

updating main!

…browser into secrashi-patch-19

bryan-brancotte

Hi @secrashi

It's a big task your started ! So please do not take badly the numerous comments. There is still work to be done, but what have already been done allows to raise new questions which is a good thing.

bryan-brancotte · 2021-05-19T08:34:21Z

js/index.js

@@ -64,6 +64,30 @@ window.onload = function() {
    }else{
        browser.current_branch(branch);
    }
+    let keys = Object.keys(sessionStorage);
+    if (keys != "IsThisFirstTime_Log_From_LiveServer") {


I removed all IsThisFirstTime_Log_From_LiveServer and the code still works, why adding it ? if it works without it, It generally means that we do not need it (unless it prevents error in the specific scenario)

bryan-brancotte · 2021-05-19T08:41:18Z

js/index.js

+        for (let key of keys) {
+            if (key != "IsThisFirstTime_Log_From_LiveServer") {
+                $(".bookmarks").append(
+                `<button id="bookClick">${key}</button>`


Element ID should/must be unique, we indeed can create multiple element with the same id, but it can have strange behavior. Use classes here : ${key}`

https://www.w3schools.com/html/html_id.asp#:~:text=The%20id%20attribute%20specifies%20a,HTML%20element.

bryan-brancotte · 2021-05-19T08:42:22Z

js/index.js

+                browser
+                .interactive_tree()
+                .cmd()
+                .selectElement(`${sessionStorage.getItem(button.innerHTML)}`, true);


I'm also new to sessionStorage vs localStorage, you should read for example https://krishankantsinghal.medium.com/local-storage-vs-session-storage-vs-cookie-22655ff75a8 In our case we want to data to be saved after the tab is closed, so we should use local storage.

bryan-brancotte · 2021-05-19T08:43:42Z

js/tree-edam-stand-alone.js

@@ -232,6 +233,9 @@ function interactive_edam_browser(){
        details += '</div>';
        details=$(details);
        details.find(".term-name-heading").text(d.data.text);
+        details.find("#bookmark").click(function () {
+            bookmark(d.data.text, d.data.data.uri);


the method bookmark ask for the branch, not the text of the node.

bryan-brancotte · 2021-05-19T08:44:34Z

js/tree-edam-stand-alone.js

@@ -754,3 +758,30 @@ function toggleFullscreen(){
        $('#go-fullscreen').show();
    }
 }
+function bookmark(branch, uri) {


I think this method should be reworked, we should not store independently each bookmark, but an array/list/dict of all bookmarked elements for the current branch.

Also, adding a third attribut indicating if we want to add or remove the bookmark would be useful.

bryan-brancotte · 2021-05-19T08:46:29Z

js/index.js

+        }
+        for (let key of keys) {
+            if (key != "IsThisFirstTime_Log_From_LiveServer") {
+                $(".bookmarks").append(


will become $("#bookmarks")

bryan-brancotte · 2021-05-19T08:49:12Z

js/tree-edam-stand-alone.js

@@ -222,6 +222,7 @@ function interactive_edam_browser(){
        details +=                 '<span>';
        details +=                 '<a title="edit this term" type="button" class="btn btn-default btn-xs pull-right" target="_blank" href="edit.html?term='+uri+'&branch='+current_branch+'"><span class="glyphicon glyphicon-pencil"></span></a>';
        details +=                 '<a title="add a child to this term" type="button" class="btn btn-default btn-xs pull-right" target="_blank" href="edit.html?parent='+uri+'&branch='+current_branch+'"><span class="glyphicon glyphicon-plus"></span></a>';
+        details +=                 `<a title="bookmark this term" type="button" class="btn btn-default btn-xs pull-right" id="bookmark" target="_blank"><span class="glyphicon glyphicon-bookmark"></span></a>`;


The status of bookmarked or not should be rendered. Maybe with a gold color, or maybe by toggling between
glyphicon glyphicon-star and glyphicon glyphicon-star-empty

bryan-brancotte · 2021-05-19T09:30:19Z

js/tree-edam-stand-alone.js

+    sessionStorage.setItem(branch, uri);
+    let keys = Object.keys(sessionStorage);
+    for (const key of keys) {
+        $("#bookClick").remove();


Removing them all to re-add them is not very elegant, but as we don't not have hundreds of them it is no big deal

bryan-brancotte · 2021-05-19T09:30:49Z

js/tree-edam-stand-alone.js

@@ -754,3 +758,30 @@ function toggleFullscreen(){
        $('#go-fullscreen').show();
    }
 }
+function bookmark(branch, uri) {
+    sessionStorage.setItem(branch, uri);
+    let keys = Object.keys(sessionStorage);


Sorting them could also be useful.

bryan-brancotte · 2021-05-19T10:11:57Z

css/index.css

@@ -215,4 +215,25 @@ body {
 .flex-column{
    flex-direction: column;
 }
+#bookClick{


The aspect of the bookClick should be reworked. I woulsd sugest smaller and more un the aspect of what is currently done for "previously seen" entries. Also the aspect should allow to un-bookmark an item.

sakshi-dhamija · 2021-09-13T17:43:15Z

Hello @bryan-brancotte, would it be okay if I take upon this PR now and complete the task?

sakshi-dhamija added 9 commits April 25, 2021 13:34

Merge pull request #9 from edamontology/main

7ca69ab

Updating Repo!

Merge pull request #10 from edamontology/main

d5cb2a0

Updating main!

Merge pull request #11 from edamontology/main

daf3e32

updating main!

Merge pull request #12 from edamontology/main

7cff863

updating main!

Merge pull request #13 from edamontology/main

4fdc96b

updating main!

Implemented bookmark functionality!

5f0c8c5

Merge branch 'main' into secrashi-patch-19

f7b762b

Removed console.log statements and minor grammatical errors.

2a9e241

Merge branch 'secrashi-patch-19' of https://github.com/secrashi/edam-…

3a529fa

…browser into secrashi-patch-19

bryan-brancotte mentioned this pull request May 19, 2021

Ways to cover the excessive empty space on the website #91

Open

bryan-brancotte requested changes May 19, 2021

View reviewed changes

bryan-brancotte added the new feature label May 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented Bookmark Functionality. #195

Implemented Bookmark Functionality. #195

sakshi-dhamija commented May 16, 2021

bryan-brancotte left a comment

bryan-brancotte May 19, 2021 •

edited

Loading

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

bryan-brancotte May 19, 2021

sakshi-dhamija commented Sep 13, 2021

Implemented Bookmark Functionality. #195

Are you sure you want to change the base?

Implemented Bookmark Functionality. #195

Conversation

sakshi-dhamija commented May 16, 2021

Checklist

Issue

Details

bryan-brancotte left a comment

Choose a reason for hiding this comment

bryan-brancotte May 19, 2021 • 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

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

sakshi-dhamija commented Sep 13, 2021

bryan-brancotte May 19, 2021 •

edited

Loading