Resolved Issue #1695: Sample values in Chart activity are not initialized

[kumar-piyush12]

Resolved: Sample values in Chart activity are not initialized #1695

Note: Only changes are done in onJournalNewInstance() and localized() of file in activities/Chart.activity/js/activity.js

I sincerely apologize for any indentation errors in activity.js. Despite my best efforts, I was unable to resolve them. As I am new to Open Source Contribution, I appreciate your understanding and am committed to improving in this area moving forward. Thank you for your consideration.

Summary of Issue: In Charts section of Desktop Sugarizer, incorrect labels such as 'EgLabel6' were showing, instead of labels of Sports names (e.g. Basketball, Tennis, Cricket, etc.)

Explanation of my approach:

1. After reviewing the ChartView.js file, it is observed that the chart labels are derived directly from the tabular data without being processed through the translation system. This behavior is identified within the computed section of the code.
   labels() { return this.tabularData.map((data) => data.x || ""); },

2. This implies the necessity to identify where tabularData is being set. Given that this is a Vue component that receives tabularData as a prop, it is crucial to examine either:

• The parent component that is responsible for setting this data, or
• activity.js, which is likely the script where the initial data is being prepared.

3. After reviewing the activity.js, it can be modified to ensure the labels are properly translated using the localization system before being passed to the chart component. This would involve utilizing the translations from the en.json and fr.json files to convert labels such as "EgLabel6" to "Chess" and "EgLabel2" to "Cricket," among others.

onJournalNewInstance() {
    const randArr = this.shuffleArray([1, 2, 3, 4, 5, 6]);
    for(let i = 0; i < 3; i++) {
        const label = "EgLabel" + randArr[i];
        this.tabularData.push({
            x: this.SugarL10n.get(label),  // This should translate but isn't working
            y: randArr[i] + 6 + "",
        })
    }
    this.pref.labels.x = this.SugarL10n.get("Sports"); 
    this.pref.labels.y = this.SugarL10n.get("Students");
    // ...
}

• Above is the problematic code in activity.js, the issue appears to be in the onJournalNewInstance method, where the labels are being created but the translation is being handled incorrectly. The problem may stem from two potential causes:
• The localization system (SugarL10n) may not be fully initialized when this code executes.
• The translation function may not be functioning as intended.

4. New Version of onJournalNewInstance() to ensure labels are properly translated:

onJournalNewInstance() {
    const randArr = this.shuffleArray([1, 2, 3, 4, 5, 6]);
    for(let i = 0; i < 3; i++) {
        const label = "EgLabel" + randArr[i];
        const translatedLabel = this.SugarL10n.get(label);
        
        // Add fallback in case translation fails
        this.tabularData.push({
            x: translatedLabel || label,
            y: randArr[i] + 6 + "",
        });
        
        // Log for debugging
        console.log(`Label: ${label}, Translated: ${translatedLabel}`);
    }
    this.pref.labels.x = this.SugarL10n.get("Sports"); 
    this.pref.labels.y = this.SugarL10n.get("Students");
    this.updateActivityTitle(this.l10n.stringChartActivity);

    const header = [this.pref.labels.x, this.pref.labels.y];
    this.$refs.csvView.updateJsonData(this.tabularData, header, true);
}

5. New Version of localized() method to ensure translations are loaded:

    localized() {
      this.SugarL10n.localize(this.l10n)
      document.getElementById('export-csv-button').title =
        this.l10n.stringexportAsCSV
      document.getElementById('export-img-button').title =
        this.l10n.stringSaveImage

      // Add this to ensure translations are ready
      if (this.tabularData.length === 0) {
        this.onJournalNewInstance()
      } else {
        // Refresh existing labels with translations
        this.tabularData = this.tabularData.map((data) => ({
          ...data,
          x: data.x.startsWith('EgLabel') ? this.SugarL10n.get(data.x) : data.x,
          y: data.y,
        }))
      }
    },

6. These changes will:

• Ensure translations are loaded prior to usage.
• Implement fallback behavior in case translations fail.
• Add logging for better translation debugging.
• Update existing labels once translations are available.
• Enhance the robustness of the translation system.

[llaske]

I sincerely apologize for any indentation errors in activity.js. Despite my best efforts, I was unable to resolve them.

You should change the settings in your IDE to avoid auto re-indent.
Only changed line should be in the PR.

[kumar-piyush12]

I sincerely apologize for any indentation errors in activity.js. Despite my best efforts, I was unable to resolve them.

You should change the settings in your IDE to avoid auto re-indent. Only changed line should be in the PR.

Thank you for helping me out, please have a look at my updated code. I hope it completely resolves the issue.

• The commit is titled as 'fix chart activity'
• Please let me know if further modifications are required.

[SnehalSrivastava27]

@kumar-piyush12 I feel there are some indentation done automatically by IDE in your new commit as well

[SnehalSrivastava27]

The line 147 to 152 which you changed should be same as there is indentation change only

[kumar-piyush12]

The line 147 to 152 which you changed should be same as there is indentation change only
Hi Snehal, thanks for the review. I again tried to work on indentation issue, and I was able to ensure that only 'changed lines' are in PR.
But in the changed lines, I am still encountering 'internal spacing issue'. I tried my best but I could not resolve it.

I manually turned 'Auto Indent' settings to 'None' in VS-code. And it still shows some spacing issues in the 'changed lines'.
I humbly request @llaske to accept my PR request, and do let me know if any other issues arise.
I have made sure that Sugarizer runs properly with these changes
Commit title: 'fixed indentation activity.js'

[llaske]

With your fix it's not longer possible to have an empty chart because of the test you've added. See below.

Capture.video.du.2025-01-09.21-49-31.webm

You should not test the data length. You should ensure that a new instance has been created.
Plus your level of indentation is wrong (one level too high).