Skip to content

Commit

Permalink
CLDR-16844 Show the Dashboard by default, and remember user preference
Browse files Browse the repository at this point in the history
-New boolean dashWanted in cldrDashContext.mjs indicates whether the user wants the Dashboard open when appropriate

-New boolean method cldrDashContext.shouldBeShown takes into account dashWanted, and whether a Special page (other than GENERAL_SPECIAL) is open

-Add code to open the Dashboard (if shouldBeShown) where needed

-Encapsulate the string, general, as cldrLoad.GENERAL_SPECIAL

-Move message text to dash_needs_locale_and_coverage in cldrText.mjs, referenced in two places

-Fix bug: effectiveName called without locale arg; console.error if it happens again

-Remove confusing checkmark (vote.png) from General Info page; move style into GeneralInfo.vue

-Fix call to getClient, await is required

-Mark getClient as async to prevent VS Code wrongly warning that await is not needed

-Remove dead code

-Comments
  • Loading branch information
btangmu committed Aug 3, 2024
1 parent e3a0dbb commit 1d29d6c
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 38 deletions.
4 changes: 2 additions & 2 deletions tools/cldr-apps/js/src/esm/cldrClient.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ const RESOLVED_ROOT = new URL(OAS3_ROOT, document.baseURI);
*
* <https://github.com/swagger-api/swagger-js/blob/master/docs/usage/tags-interface.md#openapi-v3x>
*
* @returns Promise<SwaggerClient>
* @returns {Promise<SwaggerClient>}
*/
function getClient() {
async function getClient() {
return new SwaggerClient({
url: RESOLVED_ROOT,
requestInterceptor: (obj) => {
Expand Down
4 changes: 4 additions & 0 deletions tools/cldr-apps/js/src/esm/cldrCoverage.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,14 @@ function effectiveCoverage(locale) {
*
* The effective level is the coverage level that the user has chosen, or else the organization's
* coverage level if the user has not chosen one
* @param {String} locale the locale ID
*
* @return the name, or null
*/
function effectiveName(locale) {
if (locale === undefined) {
console.error("cldrCoverage.effectiveName called without locale argument");
}
if (surveyUserCov) {
return surveyUserCov;
}
Expand Down
57 changes: 46 additions & 11 deletions tools/cldr-apps/js/src/esm/cldrDashContext.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
*/

import * as cldrDrag from "./cldrDrag.mjs";
import * as cldrLoad from "./cldrLoad.mjs";
import * as cldrNotify from "./cldrNotify.mjs";
import * as cldrStatus from "./cldrStatus.mjs";
import * as cldrVue from "./cldrVue.mjs";

import DashboardWidget from "../views/DashboardWidget.vue";
Expand All @@ -20,12 +22,37 @@ const NEIGHBOR_SECTION_ID = "VotingEtcSection";
let wrapper = null;

/**
* Is the Info Panel currently displayed?
* Is the Dashboard currently displayed?
*/
let visible = false;
let dashVisible = false;

/**
* Does the user want the Dashboard to be displayed when appropriate?
* It is hidden automatically when there is a "special" view for which
* it is inappropriate. When returning to the non-special vetting view,
* the Dashboard should be displayed again, unless the user has indicated, by clicking
* its close box, that they don't want to see it.
*/
let dashWanted = true;

function isVisible() {
return visible; // boolean
return dashVisible; // boolean
}

/**
* Should the Dashboard be shown?
*
* Default is true when called the first time.
* Subsequently, remember whether the user has left it open or closed.
*
* @returns true if the Dashboard should be shown, else false
*/
function shouldBeShown() {
if (!dashWanted) {
return false;
}
const spec = cldrStatus.getCurrentSpecial();
return !spec || spec === cldrLoad.GENERAL_SPECIAL;
}

function wireUpOpenButtons() {
Expand All @@ -39,7 +66,7 @@ function wireUpOpenButtons() {
* Create or reopen the DashboardWidget Vue component
*/
function insert() {
if (visible) {
if (dashVisible) {
return; // already inserted and visible
}
try {
Expand All @@ -61,7 +88,7 @@ function insert() {
* Show the Dashboard
*/
function show() {
if (visible) {
if (dashVisible) {
return;
}
const vote = document.getElementById(NEIGHBOR_SECTION_ID);
Expand All @@ -74,16 +101,22 @@ function show() {
for (let i = 0; i < els.length; i++) {
els[i].style.display = "none";
}
visible = true;
dashVisible = dashWanted = true;
cldrDrag.enable(vote, dash, true /* up/down */);
}
}

/**
* Hide the Dashboard
*
* @param {Boolean} userWantsHidden true if closing because user clicked close box (or equivalent),
* false if closing because switching to a "special" view where the Dashboard doesn't belong
*/
function hide() {
if (!visible) {
function hide(userWantsHidden) {
if (userWantsHidden === undefined) {
console.error("cldrDashContext.hide was called with undefined parameter");
}
if (!dashVisible) {
return;
}
const vote = document.getElementById(NEIGHBOR_SECTION_ID);
Expand All @@ -95,18 +128,19 @@ function hide() {
for (let i = 0; i < els.length; i++) {
els[i].style.display = "inline";
}
visible = false;
dashVisible = false;
dashWanted = !userWantsHidden;
}
}

function updateRow(json) {
if (visible) {
if (dashVisible) {
wrapper?.updatePath(json);
}
}

function updateWithCoverage(newLevel) {
if (visible) {
if (dashVisible) {
wrapper?.handleCoverageChanged(newLevel);
}
}
Expand All @@ -115,6 +149,7 @@ export {
hide,
insert,
isVisible,
shouldBeShown,
updateRow,
updateWithCoverage,
wireUpOpenButtons,
Expand Down
7 changes: 4 additions & 3 deletions tools/cldr-apps/js/src/esm/cldrDashData.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ function doFetch(callback) {
const locale = cldrStatus.getCurrentLocale();
const level = cldrCoverage.effectiveName(locale);
if (!locale || !level) {
fetchErr = "Please choose a locale and a coverage level first.";
fetchErr = cldrText.get("dash_needs_locale_and_coverage");
this.console.log("cldrDashData doFetch dash_needs_locale_and_coverage");
return;
}
const url = `api/summary/dashboard/${locale}/${level}`;
Expand Down Expand Up @@ -473,10 +474,10 @@ async function downloadXlsx(data, locale, cb) {

/**
* @param {string} locale locale to list for
* @returns {Array<CheckStatusSummary>}
* @returns {Promise<Array<CheckStatusSummary>>}
*/
async function getLocaleErrors(locale) {
const client = cldrClient.getClient();
const client = await cldrClient.getClient();
return await client.apis.voting.getLocaleErrors({ locale });
}

Expand Down
8 changes: 5 additions & 3 deletions tools/cldr-apps/js/src/esm/cldrEvent.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ function checkLocaleShow(element, size) {
*/
function interceptLocale() {
var name = $(this).text();
var source = $(this).attr("title");

$("input.local-search").val(name);
$("a.locName").removeClass("active");
Expand Down Expand Up @@ -434,6 +433,9 @@ function unpackMenuSideBar(json) {
cldrLoad.reloadV();
$("#left-sidebar").removeClass("active");
toggleOverlay();
if (cldrDashContext.shouldBeShown()) {
cldrDashContext.insert();
}
});

// review link
Expand All @@ -442,9 +444,9 @@ function unpackMenuSideBar(json) {
toggleOverlay();
const url = $(this).data("url");
if (url === "dashboard") {
// Note: setCurrentSpecial("general") is dubious here; it doesn't cause
// Note: setCurrentSpecial(cldrLoad.GENERAL_SPECIAL) is dubious here; it doesn't cause
// the "general" page to be loaded; it doesn't hide whatever else was displayed.
cldrStatus.setCurrentSpecial("general");
cldrStatus.setCurrentSpecial(cldrLoad.GENERAL_SPECIAL);
cldrDashContext.insert();
} else {
$("#OtherSection").hide(); // Don't hide the other section when showing the dashboard.
Expand Down
34 changes: 26 additions & 8 deletions tools/cldr-apps/js/src/esm/cldrLoad.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ import { h } from "vue";

const CLDR_LOAD_DEBUG = false;

/**
* This value for the "special" page description, as returned by cldrStatus.getCurrentSpecial(),
* corresponds to the situation in which a locale has been chosen but no section/page has been
* chosen for that locale. It corresponds to GeneralInfo.vue.
*/
const GENERAL_SPECIAL = "general";

let locmap = new LocaleMap(null); // a localemap that always returns the code
// locmap will be modified later with locmap = new LocaleMap(json.locmap)

Expand Down Expand Up @@ -110,11 +117,14 @@ function doHashChange(event) {
const changedSpecial = oldSpecial != curSpecial;
const changedPage = oldPage != trimNull(cldrStatus.getCurrentPage());
if (changedLocale || (changedSpecial && curSpecial)) {
cldrDashContext.hide();
cldrDashContext.hide(false /* userWantsHidden */);
}
if (changedLocale || changedSpecial || changedPage) {
console.log("# hash changed, (loc, etc) reloadingV..");
reloadV();
if (cldrDashContext.shouldBeShown()) {
cldrDashContext.insert();
}
} else if (
oldId != cldrStatus.getCurrentId() &&
cldrStatus.getCurrentId() != ""
Expand Down Expand Up @@ -610,8 +620,8 @@ function specialLoad(itemLoadInfo, curSpecial, theDiv) {
const special = getSpecial(curSpecial); // special is an object; curSpecial is a string
if (special && special.load) {
cldrEvent.hideOverlayAndSidebar();
if (curSpecial !== "general") {
cldrDashContext.hide();
if (curSpecial !== GENERAL_SPECIAL) {
cldrDashContext.hide(false /* userWantsHidden */);
}
cldrInfo.closePanel(false /* userWantsHidden */);
// Most special.load() functions do not use a parameter; an exception is
Expand All @@ -622,11 +632,11 @@ function specialLoad(itemLoadInfo, curSpecial, theDiv) {
);
}
special.load(curSpecial);
} else if (curSpecial !== "general") {
} else if (curSpecial !== GENERAL_SPECIAL) {
// Avoid recursion.
unspecialLoad(itemLoadInfo, theDiv);
} else {
// This will only be called if 'general' is a missing special.
// This will only be called if 'general' (GENERAL_SPECIAL) is a missing special.
handleMissingSpecial(curSpecial);
}
}
Expand All @@ -641,8 +651,8 @@ function unspecialLoad(itemLoadInfo, theDiv) {
if (CLDR_LOAD_DEBUG) {
console.log("cldrLoad.unspecialLoad: running specialLoad(general)");
}
cldrStatus.setCurrentSpecial("general");
specialLoad(itemLoadInfo, "general", theDiv);
cldrStatus.setCurrentSpecial(GENERAL_SPECIAL);
specialLoad(itemLoadInfo, GENERAL_SPECIAL, theDiv);
} else if (curId === "!") {
// TODO: clarify when and why this would happen
if (CLDR_LOAD_DEBUG) {
Expand Down Expand Up @@ -1143,7 +1153,14 @@ function flipToEmptyOther() {

function coverageUpdate() {
cldrCoverage.updateCoverage(flipper.get(pages.data));
handleCoverageChanged(cldrCoverage.effectiveName());
const curLocale = cldrStatus.getCurrentLocale();
if (!curLocale) {
console.error(
"cldrLoad.coverageUpdate called when current locale not defined"
);
return;
}
handleCoverageChanged(cldrCoverage.effectiveName(curLocale));
}

function setLoading(loading) {
Expand All @@ -1168,6 +1185,7 @@ export {
flipToEmptyOther,
flipToGenericNoLocale,
flipToOtherDiv,
GENERAL_SPECIAL,
getHash,
getLocaleDir,
getLocaleInfo,
Expand Down
4 changes: 2 additions & 2 deletions tools/cldr-apps/js/src/esm/cldrMenu.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,9 @@ function getMenusFromServer() {
return; // busted?
}
// Note: since the url has "locmap=false", we never get json.locmap or json.canmodify here
const covName = cldrCoverage.effectiveName();
const covName = cldrCoverage.effectiveName(curLocale);
cldrCoverage.updateCovFromJson(json);
if (cldrCoverage.effectiveName() !== covName) {
if (cldrCoverage.effectiveName(curLocale) !== covName) {
cldrLoad.coverageUpdate();
}
unpackMenus(json);
Expand Down
3 changes: 3 additions & 0 deletions tools/cldr-apps/js/src/esm/cldrText.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
const CLDR_TEXT_DEBUG = false;

const strings = {
dash_needs_locale_and_coverage:
"Please choose a locale and a coverage level first.",

downgradedDeletionSuccessHeader: "Success",
downgradedDeletionSuccessDetail:
"The imported votes for downgraded paths were successfully deleted",
Expand Down
4 changes: 3 additions & 1 deletion tools/cldr-apps/js/src/esm/cldrVueMap.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as cldrLoad from "./cldrLoad.mjs";

import AboutPanel from "../views/AboutPanel.vue";
import AnnouncePanel from "../views/AnnouncePanel.vue";
import AddUser from "../views/AddUser.vue";
Expand All @@ -24,7 +26,7 @@ const specialToComponentMap = {
add_user: AddUser,
auto_import: AutoImport,
downgraded: DowngradedVotes,
general: GeneralInfo,
general: GeneralInfo, // see cldrLoad.GENERAL_SPECIAL
lock_account: LockAccount,
lookup: LookUp,
menu: MainMenu,
Expand Down
6 changes: 4 additions & 2 deletions tools/cldr-apps/js/src/views/DashboardWidget.vue
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,9 @@ export default {
this.locale = cldrStatus.getCurrentLocale();
this.level = cldrCoverage.effectiveName(this.locale);
if (!this.locale || !this.level) {
this.fetchErr = "Please choose a locale and a coverage level first.";
// This sometimes happens when locale is defined but level is not yet defined,
// but it changes so quickly (the level gets defined) that it isn't visible
this.fetchErr = cldrText.get("dash_needs_locale_and_coverage");
return;
}
this.localeName = cldrLoad.getLocaleName(this.locale);
Expand Down Expand Up @@ -359,7 +361,7 @@ export default {
},
closeDashboard() {
cldrDashContext.hide();
cldrDashContext.hide(true /* userWantsHidden */);
},
abbreviate(category) {
Expand Down
22 changes: 22 additions & 0 deletions tools/cldr-apps/js/src/views/GeneralInfo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default {
specialGeneral: null,
};
},
mounted() {
const locmap = cldrLoad.getTheLocaleMap();
const bund = locmap.getLocaleInfo(cldrStatus.getCurrentLocale());
Expand All @@ -52,8 +53,23 @@ export default {
}
this.specialGeneral = cldrText.get("generalSpecialGuidance");
this.showDashboardOrButton();
},
methods: {
showDashboardOrButton() {
// If Dashboard is already visible, hide the button for opening it.
if (cldrDashContext.isVisible()) {
const els = document.getElementsByClassName("general-open-dash");
for (let i = 0; i < els.length; i++) {
els[i].style.display = "none";
}
} else if (cldrDashContext.shouldBeShown()) {
this.insertDashboard();
}
},
insertDashboard() {
cldrDashContext.insert();
},
Expand All @@ -66,4 +82,10 @@ button.general-open-dash {
/* We only want THIS button to float, not all Open Dashboard buttons. */
float: right;
}
p.special_general {
margin: 1em;
padding: 1em;
border: 2px solid gray;
}
</style>
Loading

0 comments on commit 1d29d6c

Please sign in to comment.