Skip to content

Commit

Permalink
SAK-44151 General small improvements and fixes for Lessons (sakaiproj…
Browse files Browse the repository at this point in the history 
…ect#8477)

These improvements and bug fixes include:
- toggling a css class on section headers based on if the section is open or closed
- Fixing a typo in the polls markup causing styling inconsistencies
- Lowering all of the z-index values for lessons dialogs
- Fixing styling inconsistencies in Checklist feature
- Adding tabindex to the Learning Apps iframe
- Linked checklist items are broken when imported into a new site, but are not
indicated as broken to the user
- Change checklist item duplication to use JSON instead of String manipulation
- Fix date picker when no date is set
- Fix issue with checklist item indexes
- Fix auto select multiple choice on add question load
- Keep subpage group release info on duplication / import
- Fix group releasing LTI items in Lessons
  • Loading branch information
@davidpbauer
davidpbauer authored Aug 31, 2020
1 parent a60aba9 commit d7f4930
Show file tree
Hide file tree
Showing 11 changed files with 164 additions and 80 deletions.
75 changes: 56 additions & 19 deletions ...tool/src/java/org/sakaiproject/lessonbuildertool/service/LessonBuilderEntityProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,14 @@

import org.apache.commons.fileupload.disk.DiskFileItem;
import org.apache.commons.lang3.StringUtils;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
import org.json.simple.parser.JSONParser;
import org.json.simple.parser.ParseException;
import org.jsoup.Jsoup;
import org.jsoup.select.Elements;
import org.sakaiproject.authz.api.AuthzRealmLockException;
import org.sakaiproject.authz.api.AuthzGroupService;
import org.sakaiproject.authz.api.SecurityAdvisor;
import org.sakaiproject.authz.api.SecurityService;
import org.sakaiproject.content.api.ContentHostingService;
Expand Down Expand Up @@ -510,6 +515,28 @@ else if (item.getType() == SimplePageItem.ASSESSMENT)
addAttr(doc, itemElement, "samewindow", item.isSameWindow() ? "true" : "false");

String attrString = item.getAttributeString(); //json encoded attributes

final JSONParser jsonParser = new JSONParser(); // JSON parser used to parse the attribute string into a JSON object

try {
JSONObject attributeObj = (JSONObject) jsonParser.parse(attrString); // Get full attribute string as JSON object
JSONArray checklistArr = (JSONArray) attributeObj.get("checklistItems"); // Get the checklist JSON array from the object

if (checklistArr != null && !checklistArr.isEmpty()) {
for (Object checklistObj : checklistArr) { // Iterate through each checklist item to check for links
JSONObject checklistObjJson = (JSONObject) checklistObj;
if (checklistObjJson != null && checklistObjJson.get("link") != null && (Long) checklistObjJson.get("link") > 0L) { // Null safe check if it is a linked checklist item
checklistObjJson.put("link", -2L); // This is a linked checklist item so set it to -2 to indicate link needs to be updated
}
}
}
attributeObj.put("checklistItems", checklistArr); // Update the JSON object for the JSON attribute with the modified checklist

attrString = attributeObj.toJSONString(); // Replace the attribute string with the version with the modified checklist
} catch (ParseException e) {
log.error("Exception caught while parsing checklist array. No modifications will be made to attribute string.", e.getMessage(), e);
}

if (attrString != null) {
Element attributeElement = doc.createElement("attributes");
attributeElement.setTextContent(attrString);
Expand Down Expand Up @@ -875,7 +902,7 @@ private boolean makePage(Element element, String oldServer, String siteId, Strin
item.setGroupOwned(s.equals("true"));

if (RESTORE_GROUPS) {
String groupString = mergeGroups(itemElement, "ownerGroup", siteGroups);
String groupString = mergeGroups(itemElement, "ownerGroup", siteGroups, fromSiteId);
if (groupString != null)
item.setOwnerGroups(groupString);
}
Expand All @@ -890,7 +917,7 @@ private boolean makePage(Element element, String oldServer, String siteId, Strin
// awareness comes from the other tools, enabling this produces
// inconsistent results
if (RESTORE_GROUPS) {
String groupString = mergeGroups(itemElement, "group", siteGroups);
String groupString = mergeGroups(itemElement, "groups", siteGroups, fromSiteId);
if (groupString != null)
item.setGroups(groupString);
}
Expand Down Expand Up @@ -997,31 +1024,41 @@ else if (type == SimplePageItem.ASSESSMENT)
return needFix;
}

String mergeGroups(Element itemElement, String attr, Collection<Group> siteGroups) {
String mergeGroups(Element itemElement, String attr, Collection<Group> siteGroups, String fromSiteId) {

// not currently doing this, although the code has been tested.
// The problem is that other tools don't do it. Since much of our group
// awareness comes from the other tools, enabling this produces
// inconsistent results

NodeList groups = itemElement.getElementsByTagName(attr);
String groups = itemElement.getAttribute(attr);
String groupString = null;

// translate groups from title to ID
if (groups != null && siteGroups != null) {
for (int n = 0; n < groups.getLength(); n ++) {
Element group = (Element)groups.item(n);
String title = group.getAttribute("title");
if (title != null && !title.equals("")) {
for (Group g: siteGroups) {
if (title.equals(g.getTitle())) {
if (groupString == null)
groupString = g.getId();
else
groupString = groupString + "," + g.getId();
Site oldSite = null;
try {
oldSite = siteService.getSite(fromSiteId);
} catch (Exception e) {
log.debug("site " + fromSiteId + " not found.", e);
}
// For each old group, get the corresponding new group id
if (!groups.isEmpty() && siteGroups != null && oldSite != null) {
final String[] parts = groups.split(",");
for (int n = 0; n < parts.length; n ++) {
final Group group = oldSite.getGroup(parts[n]);
final String providerID = group.getProviderGroupId();
if(providerID == null ) { // only transfer groups which are not old rosters
final String title = group.getTitle();
if (title != null && !title.equals("")) {
for (Group g : siteGroups) {
if (title.equals(g.getTitle())) {
if (groupString == null) {
groupString = g.getId();
} else {
groupString = groupString + "," + g.getId();
}
}
}
}
}
}
}
}
}

Expand Down
51 changes: 28 additions & 23 deletions ...onbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/tool/beans/SimplePageBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -867,22 +867,25 @@ public void setHidePage(boolean hide) {
}

// argument is in ISO8601 format, which has -04:00 time zone.
// if user's computer is on a different time zone, we want the UI to match
// if user's computer is on a different time zone, we want the UI to match
// Sakai. Hence we really want to handle everything as local time.
// That means we want to ignore the time zone on input
public void setReleaseDate(String date) {
if (StringUtils.isBlank(date))
this.releaseDate = null;
else
try {
// if (date.substring(22,23).equals(":"))
// date = date.substring(0,22) + date.substring(23,25);
date = date.substring(0,19);
this.releaseDate = isoDateFormat.parse(date);
} catch (Exception e) {
log.info("{}bad format releasedate {}", e, date);
}
}
public void setReleaseDate(String date) {
if (StringUtils.isBlank(date)) {
this.releaseDate = null;
} else {
try {
if(date.charAt(10) == ' '){ //the raw date that arrives from the datepicker's ISO8601 field might be missing a char [ex. "2019-08-03T13:45:00" is correct, "2019-08-03 13:45:00" is incorrect], so we will force it in here.
date = date.replace(' ', 'T');
}
date = date.substring(0,19);
this.releaseDate = isoDateFormat.parse(date);
} catch (Exception e) {
log.error("{}bad format releasedate {}", e, date);
this.releaseDate = null;
}
}
}

public Date getReleaseDate() {
return this.releaseDate;
Expand Down Expand Up @@ -3617,13 +3620,9 @@ public String getReleaseString(SimplePageItem i, Locale locale) {
if (i.getAttribute("multimediaUrl") != null)
return getLBItemGroups(i); // for all native LB objects
return getResourceGroups(i, nocache); // responsible for caching the result
// throws IdUnusedException if necessary
case SimplePageItem.BLTI:
entity = bltiEntity.getEntity(i.getSakaiId());
if (entity == null || !entity.objectExists())
throw new IdUnusedException(i.toString());
// fall through: groups controlled by LB
// for the following items we don't have non-LB items so don't need itemunused
case SimplePageItem.BLTI: // Groups always managed in lessons
case SimplePageItem.TEXT:
case SimplePageItem.RESOURCE_FOLDER:
case SimplePageItem.CHECKLIST:
Expand Down Expand Up @@ -5151,10 +5150,16 @@ else if (item.getType() == SimplePageItem.PAGE) {
return false;
break;
case SimplePageItem.BLTI:
if (bltiEntity != null)
entity = bltiEntity.getEntity(item.getSakaiId());
if (entity == null || entity.notPublished())
return false;
if (bltiEntity != null) {
entity = bltiEntity.getEntity(item.getSakaiId());
}
if (entity == null || entity.notPublished()) {
return false;
} else {
// After checking that it exists reset to null so that groups are
// checked internal to Lessons
entity = null;
}
}
}
} finally {
Expand Down
13 changes: 11 additions & 2 deletions ...der/tool/src/java/org/sakaiproject/lessonbuildertool/tool/producers/ShowPageProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3445,9 +3445,18 @@ else if(questionStatus == Status.FAILED)

UIVerbatim.make(row, "select-checklistitem-linked-details", tooltipContent);
}
UIOutput.make(row, "select-checklistitem-name", checklistItemName).decorate(new UIStyleDecorator("checklist-checkbox-label"));

} else if (checklistItem.getLink() < -1L) { // getLink will give out -2 for items that were once linked but broke during site duplication.
input.decorate(new UIDisabledDecorator(true));
UIOutput.make(row, "select-checklistitem-link-broken");
String toolTipMessage = "simplepage.checklist.external.link.details.broken";
String tooltipContent = messageLocator.getMessage(toolTipMessage);
UIVerbatim.make(row, "select-checklistitem-linked-details", tooltipContent);
UIOutput.make(row, "select-checklistitem-name", checklistItemName).decorate(new UIStyleDecorator("checklist-checkbox-label link-broken"));
} else {
UIOutput.make(row, "select-checklistitem-name", checklistItemName).decorate(new UIStyleDecorator("checklist-checkbox-label"));
}

UIOutput.make(row, "select-checklistitem-name", checklistItemName).decorate(new UIStyleDecorator("checklist-checkbox-label"));
index++;
}
}
Expand Down
1 change: 1 addition & 0 deletions lessonbuilder/tool/src/resources/messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ simpleapge.checklist.edit_externalLink=Edit external link
simplepage.checklist.external.unlink=Remove Link
simplepage.checklist.external.link.details.incomplete=This item is linked to '<strong>{}</strong>' and will be automatically checked once '<strong>{}</strong>' is completed.
simplepage.checklist.external.link.details.complete=This item is linked to the complete item: '<strong>{}</strong>'.
simplepage.checklist.external.link.details.broken=Edit this checklist to re-link all items.
simplepage.checklist.external.link.details.notvisible=This linked item is currently not visible.
simplepage.checklist.external.link.hidden=Name is not available
simplepage.checklist.external.link.info=It is only possible to link to required items.
Expand Down
4 changes: 4 additions & 0 deletions lessonbuilder/tool/src/webapp/css/Simplepagetool.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,10 @@ div[role="dialog"] {
margin: 10px 0
}

textarea.shortAnswerInput {
width: 100%;
}

.deleteCandidate td:first-child {
background: transparent url('/library/image/silk/delete.png') 2px 3px no-repeat;
}
Expand Down
27 changes: 20 additions & 7 deletions lessonbuilder/tool/src/webapp/css/checklist.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,32 +84,40 @@ p.edit-group {
}
.checklist-checkbox ~ span.checklist-checkbox-label {
background-image: url("../images/gray_unchecked.png");
background-position: left center;
background-position: left top 8px;
background-repeat: no-repeat;
}
.checklist-checkbox:checked ~ span.checklist-checkbox-label {
background-image: url("../images/green_checkmark.png");
background-position: left center;
background-position: left top 8px;
background-repeat: no-repeat;
}
.checklist-checkbox-label {
padding: 0.5em 0.5em 0.5em 22px;
display: inline-block;
vertical-align: top;
}
label.checklistLabel.disabled:hover input.checklist-checkbox ~ span.checklist-checkbox-label {
label.checklistLabel.disabled:hover input.checklist-checkbox ~ span.checklist-checkbox-label, label.checklistLabel.disabled input.checklist-checkbox ~ span.link-broken {
background-image: none;
cursor: not-allowed;
vertical-align: top;
padding-bottom: 13px;
}
span.link-broken-icon {
line-height: .5em;
padding-top: 2.5%;
}
label.checklistLabel:hover .checklist-checkbox ~ span.checklist-checkbox-label {
background-image: url("../images/gray_checkmark.png");
background-position: left center;
background-position: left top 8px;
background-repeat: no-repeat;
cursor: pointer;
}
label.checklistLabel:hover .checklist-checkbox:checked ~ span.checklist-checkbox, label.checklistLabel:hover .checklist-checkbox:checked ~ span.checklist-checkbox-label {
background-image: url("../images/green_checkmark.png");
}
.checklistLabel {
line-height: 2em;
line-height: 1.5em;
}
.hideNameCheckbox {
margin: 1em .5em 0 4em !important;
Expand Down Expand Up @@ -157,10 +165,15 @@ legend.no-border {
display: none;
}

#additionalSettings h3.checklist-settings-header {
padding-left: calc(.5em + 16px);
}

.checklistLabel.disabled:hover .externally-linked {
margin-right: -19px;
.checklistLabel.disabled:hover .externally-linked, .link-broken {
margin-right: -22px;
display: inline-block;
vertical-align: 20px;
margin-top: 8px;
}

.tooltip-content {
Expand Down
12 changes: 10 additions & 2 deletions lessonbuilder/tool/src/webapp/js/checklist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
modal: true,
resizable: false,
draggable: false
}).parent('.ui-dialog').css('zIndex', 150000);
}).parent('.ui-dialog').css('zIndex', 900);

$("#externalLink-dialog-close").click(function(e){
e.preventDefault();
Expand Down Expand Up @@ -78,7 +78,15 @@
checklist.addChecklistItem = function () {
var clonedAnswer = $("#copyableChecklistItemDiv").clone(true);
clonedAnswer.show();
var num = $("#createdChecklistItems").find("li").length + 1; // Should be currentNumberOfChecklistItems + 1
var num = 0;
var itemNamesOnScreen = document.getElementsByClassName('checklist-item-name');
for(var count=0; count<itemNamesOnScreen.length; count++){ //observe all the named items currently onscreen to try and get the largest index
var nameNow = itemNamesOnScreen[count].name.replace('checklist-item-name','0'); //get the index number of the current item alone, adding 0 just in case there isn't one
if(parseInt(nameNow) > num){ //if the index of the current item is larger than the old one, we have a new highest number.
num = parseInt(nameNow);
}
}
num = num + 1; //num should be incremented by 1 over the largest existing index.

clonedAnswer.find(".checklist-item-id").val("-1");
clonedAnswer.find(".checklist-item-name").val("");
Expand Down
Loading

0 comments on commit d7f4930

Please sign in to comment.