EzerIT / BibleOL

Web-based instruction in Biblical Hebrew and Greek
Other
25 stars 16 forks source link

Clause feature "linkage" cannot be blocked from showing #28

Closed oliverglanz closed 6 months ago

oliverglanz commented 7 months ago

When creating a clause exercise one cannot choose "Don't show" for the feature "linkage".

image

The consequence is that some answers can be revealed when the "MyView" button is being used. The below screenshot shows the linkage information "Clause:Relative" while the student is supposed to provide the correct answer for the clause relation (which would be "relative")

image

Solution: Adding all clause features to the exercise "Feature" list to allow for choosing "show" options.

tmccormack165 commented 7 months ago

Thank you @oliverglanz, I will work on this. Thank you for the solution, I hope to have this implemented by tomorrow.

tmccormack165 commented 7 months ago

Skipping conditions to add features to the feature table, but I do not see a feature called "Linkage":

Screenshot 2023-12-21 at 9 42 56 AM
oliverglanz commented 7 months ago

I think that is a feature that is not made accessible via the GUI. @oz1cz might have some more information on this.

tmccormack165 commented 7 months ago

Hi @oliverglanz , does this problem pertain to exercises related to Clauses, or is it exclusively a problem with Clause Atom exercises, or both?

oz1cz commented 7 months ago

The "Linkage" feature is available in the GUI. If you make an exercise about, for example, phrases, you will be able to set "Don't show" for Linkage under "Clause". If, however, you make an exercise about clauses, for some reason the ability to set "Don't show" for linkage disappears.

This appears to be a genuine bug. Unfortunately, I don't know what causes it. I can look into it if you want me to, but not this side of Christmas.

oz1cz commented 7 months ago

Yes, @tmccormack165, and the "TYPE" means that it requires special handling. The name of the feature is code and its format is text. This is the only feaure of this sort, so it's kind of special. (See section 9.1.4 and 21.4.1 in the Technical Documentation.)

tmccormack165 commented 7 months ago

Thank you @oz1cz

In response to deleted: "Thank you @oz1cz, I am hoping to solve it today because I think it would be a great opportunity for me to solve a genuine problem on the client code, I know that code_TYPE_text is the true feature in question and "Linkage" is the friendly name."

tmccormack165 commented 7 months ago

Is it possible that a GrammarFeature is automatically given true values for ignoreSelect, ignoreShow, and ignoreRequest?

Screenshot 2023-12-22 at 12 07 48 PM
oz1cz commented 7 months ago

I don't think so. But if you call getFeatureSetting() with a feature like _code_TYPEtext, it will return the feature setting for code. You can check the implementation of getFeatureSetting() in ts/configuration.ts.

tmccormack165 commented 7 months ago

Yes, thanks, but when I change the setting for code I get valueType is undefined in the console. I am not sure what that is in reference to.

Screenshot 2023-12-22 at 1 43 42 PM
oz1cz commented 7 months ago

I'm not sure what's happening here, unfortunately. I would need to debug it myself.

However, I have a hunch that you're looking in the wrong direction. If you want to display a "Don't show" button, then ignoreSelect, ignoreShow, and ignoreRequest are not the items that govern that. You may want take a look at the ButtonAndLabel class in paneltemplquizfeatures.ts. This is the class that is responsible for creating the Show/Request/Don't care/Don't show/Multiple choice buttons in the quiz creation dialogue.

Take a look at line 448 in paneltemplquizfeatures.ts. This is where a ButtonAndLabel object is created. I would look there to see why _code_TYPEtext does not cause a ButtonAndLabel object to be created.

But I could be wrong. Maybe I'm just sending you on a wild goose chase.

tmccormack165 commented 7 months ago

This solution only works for Clause Atom exercises

In reference to pull request: #30

I am able to add the Linkage feature to the feature panel:

Screenshot 2023-12-23 at 12 04 25 PM

This setting works when running an exercise:

Screenshot 2023-12-23 at 12 05 27 PM

My methodology:

  1. I add a condition to push Linkage (code_TYPE_text) to the keylist of the feature panel in paneltemplquizfeatures.ts:

    // Go through all features and identify the ones to include in the panel
    for (let featName in getObjectSetting(otype).featuresetting) {
    // special case for "Linkage" feature
    if(featName === 'code_TYPE_text') { 
        keylist.push(featName);
        continue;  
    }
    
    // Ignore features marked to be ignored, unless they belong to a SentenceGrammar
    if (getFeatureSetting(otype, featName).ignoreShow
        && getFeatureSetting(otype, featName).ignoreRequest
        && (sg===null || !sg.containsFeature(featName)))
        continue;
    
    // Ignore features of type 'url'
    if (typeinfo.obj2feat[otype][featName]==='url')
        continue;
    
    // Ignore the genuine feature already presented as "visual"
    if (hasSurfaceFeature && featName===configuration.surfaceFeature)
        continue;
    keylist.push(featName);
    }
  2. I also include code_TYPE_text to the objectSettings of the database specification file ETCBC4.db.json:

    "clause_atom": {
    "mayselect": true,
    "featuresetting": {
        "code": {
            "ignoreSelect": true,
            "ignoreShow": true,
            "ignoreRequest": true
        },
        "code_TYPE_text": {
    
        },  
        "dist": {
            "ignoreSelect": true,
            "ignoreShow": true,
            "ignoreRequest": true
        } ...
    }
    }
  3. I also edit the settings in paneltemplquizfeatures.ts to allow for making a 'Don't Show' button for Linkage:
    // Loop through the relevant features and create radio buttons for each
    for (let ix=0; ix<keylist.length; ++ix) {
    let featName : string = keylist[ix]; // Feture name
    if(featName === 'code_TYPE_text') {
        var canShow = false;
        var canRequest = false;
        var canDisplayGrammar = true;
    }
    else {
        var canShow = !getFeatureSetting(otype, featName).ignoreShow;
        var canRequest = !getFeatureSetting(otype, featName).ignoreRequest;
        var canDisplayGrammar = sg!==null && sg.containsFeature(featName);
    }
    let bal = new ButtonsAndLabel(getFeatureFriendlyName(otype, featName),                     // The localized name of the feature  
                                  featName,                                                    // The Emdros name of the feature     
                                  otype,                                                       // The Emdros object type             
                                  useSavedFeatures ? ptqf.getSelector(featName) : ButtonSelection.DONT_CARE, // Initially selected radio button
                                  ptqf.getHideFeatures(featName),                              // List of feature values to hide from student
                                  Boolean(getFeatureSetting(otype,featName).alternateshowrequestSql), // Can multiple choice be used?       
                                  canShow,                             // Can this be a display feature?     
                                  canRequest,                          // Can this be a request feature?     
                                  canDisplayGrammar);                  // Can this be a "don't show" feature?
    this.allBAL.push(bal);
    table.append(bal.getRow());
    }
  4. I run make to compile the javascript and update the database via the ugly .json file
oz1cz commented 6 months ago

This seems like a sensible way to solve the problem. Hard coding a feature into the TypeScript code is unfortunate, but since the alternative requires a considerable amount of reprogramming, I think this is the best way to do it.

I do, however, have two comments:

1) Linkage (feature "code_TYPE_tex") is not the only feature with the problem. Syntactic code (feature "code") has the same problem and should be treated together with code_TYPE_text. In fact, "code" and "code_TYPE_text" provide exactly the same information, but in numeric or textual format.

2) The code

if(featName === 'code_TYPE_text')

should probably be changed to

if (configuration.databaseName === 'ETCBC4' && otype === 'clause_atom' && (featName === 'code_TYPE_text' || featName === 'code'))

to make sure the special handling is not triggered by another feature named "code" or "code_TYPE_text", possibly in another database.

Note: I have not tested this extended if-statement.

tmccormack165 commented 6 months ago

Hi @oliverglanz , does this problem pertain to exercises related to Clauses, or is it exclusively a problem with Clause Atom exercises, or both?

tmccormack165 commented 6 months ago

I have solved this issue by adding a method in the PanelForOneType class called addManualButton(). The solution is in this commit

addManualButton:

private addManualButton(real_name:string, friendly_name:string, otype:string, useSavedFeatures:boolean, get_selector:number, get_hide_features:string[]): JQuery {

        let useDropDown = false;        // Can multiple choice be used?
        let canShow = false;            // Can this be a display feature?
        let canRequest = false;         // Can this be a request feature?
        let canDisplayGrammar = true;   // Can this be a "don't show" feature?

        let new_bal = new ButtonsAndLabel(friendly_name,
            real_name,
            otype,
            useSavedFeatures ? get_selector : ButtonSelection.DONT_CARE,
            get_hide_features,
            useDropDown, 
            canShow, 
            canRequest, 
            canDisplayGrammar);

        // Add the new button and label to allBAL
        this.allBAL.push(new_bal);
        return new_bal.getRow();
    }

The result of this addition is an easy framework for adding any feature to the feature panel regardless of Object Type:

Screenshot 2024-01-07 at 1 07 07 AM

The corresponding features are then greyed out in the Grammar Selection Box:

Screenshot 2024-01-07 at 1 08 55 AM
oliverglanz commented 6 months ago

Thanks Timothy for having solved this!