Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLDR-16844 Show the Dashboard by default, and remember user preference #3934

Merged
merged 6 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions tools/cldr-apps/js/src/esm/cldrClient.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ 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() {
return new SwaggerClient({
async function getClient() {
return await SwaggerClient({
url: RESOLVED_ROOT,
requestInterceptor: (obj) => {
// add the session header to each request
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
6 changes: 3 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,7 @@ 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");
return;
}
const url = `api/summary/dashboard/${locale}/${level}`;
Expand Down Expand Up @@ -473,10 +473,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could integrate this better, but this is sufficient for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really all of these keys could benefit from encapsulation. I'm not sure what how/where, though... using cldrLoad.GENERAL_SPECIAL in place of general for the key isn't allowed in the initializer, but we could have initialization code like

specialToComponentMap[cldrLoad.GENERAL_SPECIAL] = GeneralInfo;

and likewise for the other keys...

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";
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this and not a reactive vue property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was based on similar code in cldrDashContext.mjs. I can change it to a property...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to a property

} 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
Loading