Skip to content

Commit

Permalink
for #6642 code improvements
Browse files Browse the repository at this point in the history
Moved selectedPlots from a saved model value to a plot variable. This is really just local plot state, and only persistent values should end up on the models (and also be present in the model schema).

The existing plotVisibility code was confusing becuase it used "false" as visible, and was also keying off the model name which is static for a plot instance. I changed it so plotVisibility[i] is true if the plot is visible.

$scope.plotLabels() should have been a normal function, rather than on $scope because it is used internally and not referenced from templated html.

$scope.plotLabels() could get replaced with the existing getPlotLabels().

The code now resets plotVisibility() if the current plot labels don't match selectedPlotLabels.

Replaced JSON.stringify() comparison with appState.deepEquals().
  • Loading branch information
moellep committed Feb 2, 2024
1 parent 2eaecb5 commit 7ed8aed
Showing 1 changed file with 17 additions and 55 deletions.
72 changes: 17 additions & 55 deletions sirepo/package_data/static/js/sirepo-plotting.js
Original file line number Diff line number Diff line change
Expand Up @@ -3524,11 +3524,12 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
},
templateUrl: '/static/html/plot2d.html' + SIREPO.SOURCE_CACHE_KEY,
controller: function($scope, $element) {
var includeForDomain = [];
var childPlots = {};
var scaleFunction;
var plotVisibility = {};
let childPlots = {};
let dynamicYLabel = false;
let includeForDomain = [];
let plotVisibility = {};
let scaleFunction;
let selectedPlotLabels = [];

// for built-in d3 symbols - the units are *pixels squared*
var symbolSize = 144.0;
Expand Down Expand Up @@ -3600,14 +3601,6 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
return true;
}

function cachedPlotVisibility(pIndex, modelName) {
plotVisibility[modelName] = plotVisibility[modelName] || {};
if (! plotVisibility[modelName].hasOwnProperty(pIndex)) {
plotVisibility[modelName][pIndex] = false;
}
return plotVisibility[modelName][pIndex];
}

function createLegend() {
const plots = $scope.axes.y.plots;
var legend = $scope.select('.sr-plot-legend');
Expand All @@ -3631,7 +3624,7 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
.attr('y', 17 + count * 20)
.text(vIconText(true))
.on('click', function() {
togglePlot(i, $scope.modelName);
togglePlot(i);
$scope.$applyAsync();
});
itemWidth = item.node().getBBox().width;
Expand Down Expand Up @@ -3757,12 +3750,10 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
});
}

function togglePlot(pIndex, modelName) {
function togglePlot(pIndex) {
setPlotVisible(pIndex, ! isPlotVisible(pIndex));
updateYLabel();
if (plotVisibility) {
plotVisibility[modelName][pIndex] = ! plotVisibility[modelName][pIndex];
}
plotVisibility[pIndex] = ! plotVisibility[pIndex];
}

function updateYLabel() {
Expand Down Expand Up @@ -3891,17 +3882,6 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
});
};

$scope.plotLabels = (plots) => {
let res = [];
if (plots == null) {
return res;
}
for (let plot of plots) {
res.push(plot.label);
}
return res;
};

$scope.load = function(json) {
if (! json.plots && ! json.points) {
//TODO(pjm): plot may be loaded with { state: 'canceled' }?
Expand Down Expand Up @@ -4092,39 +4072,21 @@ SIREPO.app.directive('parameterPlot', function(appState, focusPointService, layo
: 20;
$scope.margin.bottom = 50 + 20 * legendCount;
$scope.updatePlot(json);

if (! appState.deepEquals(getPlotLabels(), selectedPlotLabels)) {
plotVisibility = {};
selectedPlotLabels = getPlotLabels();
}
plots.forEach(function(plot, ip) {
// make sure everything is visible when reloading
includeDomain(ip, true);
setPlotVisible(ip, true);
});
updateYLabel();
plots.forEach(function(plot, i) {
if (cachedPlotVisibility(i, $scope.modelName)) {
setPlotVisible(i, ! isPlotVisible(i));
if (! plotVisibility.hasOwnProperty(ip)) {
plotVisibility[ip] = true;
}
setPlotVisible(ip, plotVisibility[ip]);
});

function handlePlotSelection() {
const c = $scope.plotLabels(plots);
const p = appState.models[$scope.modelName].selectedPlots || c;
if (JSON.stringify(c) != JSON.stringify(p)) {
plots.forEach((plot, i) => {
plotVisibility[$scope.modelName][i] = false;
});
appState.models[$scope.modelName].selectedPlots = c;
appState.saveChanges($scope.modelName);
}
}
handlePlotSelection();
updateYLabel();
};

$scope.$on($scope.modelName + '.changed', () => {
// $scope.axes.y.plots is actually the plot selection prior to .changed
// caching them here allows handlePlotSelection to check previous vs current
appState.models[$scope.modelName].selectedPlots = $scope.plotLabels($scope.axes.y.plots);
}
);

$scope.recalculateYDomain = function() {
var ydom;
var xdom = $scope.axes.x.scale.domain();
Expand Down

0 comments on commit 7ed8aed

Please sign in to comment.