-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented Bookmark Functionality. #195
base: main
Are you sure you want to change the base?
Changes from all commits
7ca69ab
d5cb2a0
daf3e32
7cff863
4fdc96b
5f0c8c5
f7b762b
2a9e241
3a529fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,6 +64,30 @@ window.onload = function() { | |
}else{ | ||
browser.current_branch(branch); | ||
} | ||
let keys = Object.keys(sessionStorage); | ||
if (keys != "IsThisFirstTime_Log_From_LiveServer") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed all |
||
if ($(".bookmarks").length == 0) { | ||
$(".parentContainer").append("<div class='bookmarks'></div>"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of dynamically adding it, add it in index.html where you want. Also consider adding |
||
} | ||
for (let key of keys) { | ||
if (key != "IsThisFirstTime_Log_From_LiveServer") { | ||
$(".bookmarks").append( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will become |
||
`<button id="bookClick">${key}</button>` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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}` |
||
); | ||
} | ||
} | ||
|
||
const buttons = document.querySelectorAll("#bookClick"); | ||
for (const button of buttons) { | ||
button.addEventListener("click", function () { | ||
browser.interactive_tree().cmd().clearSelectedElements(false); | ||
browser | ||
.interactive_tree() | ||
.cmd() | ||
.selectElement(`${sessionStorage.getItem(button.innerHTML)}`, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
}); | ||
} | ||
} | ||
$("input[name='show-detail']").prop("checked" , (localStorage.getItem("show-detail")||"true") == "true"); | ||
$("input[name='show-community-usage']").prop("checked" , (localStorage.getItem("show-community-usage")||"false") == "true"); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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>`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The status of bookmarked or not should be rendered. Maybe with a gold color, or maybe by toggling between |
||
details += '</span>'; | ||
details += '</h4>'; | ||
details += '</div>'; | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the method |
||
}); | ||
return details; | ||
} | ||
|
||
|
@@ -754,3 +758,30 @@ function toggleFullscreen(){ | |
$('#go-fullscreen').show(); | ||
} | ||
} | ||
function bookmark(branch, uri) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
sessionStorage.setItem(branch, uri); | ||
let keys = Object.keys(sessionStorage); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorting them could also be useful. |
||
for (const key of keys) { | ||
$("#bookClick").remove(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
for (const key of keys) { | ||
if (key != "IsThisFirstTime_Log_From_LiveServer") { | ||
if ($(".bookmarks").length == 0) { | ||
$(".parentContainer").append("<div class='bookmarks'></div>"); | ||
} | ||
$(".bookmarks").append( | ||
`<button id="bookClick">${key}</button>` | ||
); | ||
} | ||
} | ||
const buttons = document.querySelectorAll("#bookClick"); | ||
for (const button of buttons) { | ||
button.addEventListener("click", function () { | ||
browser.interactive_tree().cmd().clearSelectedElements(false); | ||
browser | ||
.interactive_tree() | ||
.cmd() | ||
.selectElement(`${sessionStorage.getItem(button.innerHTML)}`, true); | ||
}); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.