Skip to content

Commit

Permalink
Simplify the code that deals with modals
Browse files Browse the repository at this point in the history 
We use modals in a few places, we can make their code simpler.
This doesn't really change much but it removes a few lines of code and
improves the consistency of the code.

- the parent is always the container, it never needs to be passed in
  or be called in the d3 style, like `container.call(fn)`
- `show()` always needs to be called first to create the modal
- content within the rapid-styled modals doesn't need an extra
  nested div for `rapid-stack`
  • Loading branch information
@bhousel
bhousel committed Dec 5, 2024
1 parent 7910c61 commit d0fec24
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 153 deletions.
24 changes: 13 additions & 11 deletions css/80_app_fb.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,6 @@ button.rapid-features.layer-off use {
font-size: 16px;
}
.modal.rapid-modal.modal-whatsnew .rapid-checkbox-label {
display: flex;
align-items: center;
width: 100%;
}
.modal.rapid-modal.modal-whatsnew .rapid-checkbox-text {
Expand All @@ -494,23 +492,22 @@ button.rapid-features.layer-off use {

/* dark scrollbars */
.modal.rapid-modal {
scrollbar-width: thin;
scrollbar-color: #aaa #555;
scrollbar-width: thin;
}
.modal.rapid-modal ::-webkit-scrollbar {
width: 10px;
width: 10px;
}
.modal.rapid-modal ::-webkit-scrollbar-track {
background: #444;
background: #444;
}
.modal.rapid-modal ::-webkit-scrollbar-thumb {
background-color: #da26d3;
border-radius: 6px;
border: 0;
background-color: #da26d3;
border-radius: 6px;
border: 0;
}
.modal.rapid-modal ::-webkit-scrollbar-thumb:hover {
background-color: #c6d;
cursor: pointer;
background-color: #c6d;
cursor: pointer;
}


Expand Down Expand Up @@ -624,6 +621,11 @@ button.rapid-features.layer-off use {
align-items: center;
}

.rapid-checkbox-label {
display: flex;
align-items: center;
}

.rapid-colorpicker-label {
display: flex;
align-items: center;
Expand Down
5 changes: 2 additions & 3 deletions modules/core/UiSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export class UiSystem extends AbstractSystem {
* render
* Renders the Rapid user interface into the main container.
* Note that most `render` functions accept a parent selection,
* this is the only one that doesn't need it - `$container` is the parent.
* this one doesn't need it - `$container` is always the parent.
*/
render() {
const context = this.context;
Expand All @@ -241,8 +241,7 @@ export class UiSystem extends AbstractSystem {
.attr('dir', l10n.textDirection())
.call(this.Fullscreen.render)
.call(this.Defs.render)
.call(this.Sidebar.render)
.call(this.Shortcuts.render);
.call(this.Sidebar.render);

// .main-content
// Contains the map and everything floating above it, such as toolbars, etc.
Expand Down
48 changes: 21 additions & 27 deletions modules/ui/UiRapidCatalog.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { EventEmitter } from 'pixi.js';
import { select, selection } from 'd3-selection';
import { select } from 'd3-selection';
import { Extent } from '@rapid-sdk/math';
import { marked } from 'marked';

Expand Down Expand Up @@ -39,7 +39,6 @@ export class UiRapidCatalog extends EventEmitter {
this.$parentModal = $parentModal;
this.$wrap = null;
this.$modal = null;
this.$content = null;

// Ensure methods used as callbacks always have `this` bound correctly.
// (This is also necessary when using `d3-selection.call`)
Expand All @@ -62,14 +61,13 @@ export class UiRapidCatalog extends EventEmitter {
* show
* This shows the catalog if it isn't alreaday being shown.
* For this kind of popup component, must first `show()` to create the modal.
* @param {d3-selection} $parent - A d3-selection to a HTMLElement that this component should render itself into
*/
show($parent) {
show() {
const context = this.context;
const $container = context.container();
const $container = context.container(); // $container is always the parent for a modal

// Unfortunately `uiModal` is written in a way that there can be only one at a time.
// So we have to roll our own modal here instead of just creating a second `uiModal`.
// So we need to roll our own modal here instead of just creating a second `uiModal`.
const $shaded = $container.selectAll('.shaded'); // container for the existing modal
if ($shaded.empty()) return;
if ($shaded.selectAll('.modal-catalog').size()) return; // catalog modal exists already
Expand Down Expand Up @@ -121,40 +119,32 @@ export class UiRapidCatalog extends EventEmitter {

$$modal
.append('div')
.attr('class', 'rapid-stack content');
.attr('class', 'content rapid-stack');

// update
this.$wrap = $wrap = $wrap.merge($$wrap);
this.$modal = $wrap.selectAll('.modal-catalog');
this.$content = this.$modal.selectAll('.content');

this.$modal
.transition()
.style('opacity', 1);

this.render($parent);
this.render();
}


/**
* render
* Accepts a parent selection, and renders the content under it.
* (The parent selection is required the first time, but can be inferred on subsequent renders.)
* @param {d3-selection} $parent - A d3-selection to a HTMLElement that this component should render itself into
* Renders the content inside the modal.
* Note that most `render` functions accept a parent selection,
* this one doesn't need it - `$modal` is always the parent.
*/
render($parent = this.$parent) {
if ($parent instanceof selection) {
this.$parent = $parent;
} else {
return; // no parent - called too early?
}
render() {
if (!this.$modal) return; // need to call `show()` first to create the modal.

const context = this.context;
const l10n = context.systems.l10n;

if (!this.$modal) return; // need to call `show()` first to create the modal.

const $content = this.$content;
const $content = this.$modal.selectAll('.content');

/* Header section */
let $header = $content.selectAll('.rapid-catalog-header')
Expand Down Expand Up @@ -330,19 +320,22 @@ export class UiRapidCatalog extends EventEmitter {
/**
* renderDatasets
* Renders datasets details into the `.rapid-catalog-datasets-section` div.
* @param {d3-selection} $container - A d3-selection to a HTMLElement that this component should render itself into
* @param {d3-selection} $selection - A d3-selection to a HTMLElement that this component should render itself into
*/
renderDatasets($container) {
renderDatasets($selection) {
if (!this.$modal) return; // need to call `show()` first to create the modal.

const context = this.context;
const assets = context.systems.assets;
const l10n = context.systems.l10n;
const storage = context.systems.storage;
const $content = this.$modal.selectAll('.content');

const showPreview = storage.getItem('rapid-internal-feature.previewDatasets') === 'true';
const esri = context.services.esri;

const $status = $container.selectAll('.rapid-catalog-datasets-status');
const $results = $container.selectAll('.rapid-catalog-datasets');
const $status = $selection.selectAll('.rapid-catalog-datasets-status');
const $results = $selection.selectAll('.rapid-catalog-datasets');

if (!esri || (Array.isArray(this._datasetInfo) && !this._datasetInfo.length)) {
$results.classed('hide', true);
Expand Down Expand Up @@ -516,9 +509,10 @@ export class UiRapidCatalog extends EventEmitter {
.classed('secondary', d => this.isDatasetAdded(d))
.text(d => this.isDatasetAdded(d) ? l10n.t('rapid_feature_toggle.esri.remove') : l10n.t('rapid_feature_toggle.esri.add_to_map'));

// update the count
const numShown = this._datasetInfo.filter(d => !d.filtered).length;
const gt = (count > MAXRESULTS && numShown === MAXRESULTS) ? '>' : '';
this.$content.selectAll('.rapid-catalog-filter-results')
$content.selectAll('.rapid-catalog-filter-results')
.text(l10n.t('rapid_feature_toggle.esri.datasets_found', { num: `${gt}${numShown}` }));
}

Expand Down
60 changes: 25 additions & 35 deletions modules/ui/UiRapidDatasetToggle.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { select, selection } from 'd3-selection';
import { select } from 'd3-selection';
import { marked } from 'marked';

import { icon } from './intro/helper.js';
Expand Down Expand Up @@ -43,8 +43,6 @@ export class UiRapidDatasetToggle {
this.ColorPicker = null;

// D3 selections
this.$parent = null;
this.$content = null;
this.$modal = null;

// Ensure methods used as callbacks always have `this` bound correctly.
Expand All @@ -68,53 +66,45 @@ export class UiRapidDatasetToggle {
* show
* This shows the datataset modal if it isn't already being shown.
* For this kind of popup component, must first `show()` to create the modal.
* Accepts a parent selection, and renders the content under it.
* @param {d3-selection} $parent - A d3-selection to a HTMLElement that this component should render itself into
*/
show($parent) {
show() {
const context = this.context;
const isShowing = $parent.selectAll('.shaded').size();
const $container = context.container(); // $container is always the parent for a modal

if (!isShowing) {
this.$modal = uiModal($parent);
const isShowing = $container.selectAll('.shaded').size();
if (isShowing) return; // a modal is already showing

this.$modal.select('.modal')
.attr('class', 'modal rapid-modal');
this.$modal = uiModal($container);

this.ColorPicker = uiRapidColorpicker(context, this.$modal)
.on('change', this.changeColor);
this.$modal.select('.modal')
.attr('class', 'modal rapid-modal');

this.$content = this.$modal.select('.content')
.append('div')
.attr('class', 'rapid-stack');
}
this.ColorPicker = uiRapidColorpicker(context, this.$modal)
.on('change', this.changeColor);

this.$modal.select('.content')
.attr('class', 'content rapid-stack');

this.render($parent);
this.render();
}


/**
* render
* Accepts a parent selection, and renders the content under it.
* (The parent selection is required the first time, but can be inferred on subsequent renders.)
* @param {d3-selection} $parent - A d3-selection to a HTMLElement that this component should render itself into
* Renders the content inside the modal.
* Note that most `render` functions accept a parent selection,
* this one doesn't need it - `$modal` is always the parent.
*/
render($parent = this.$parent) {
if ($parent instanceof selection) {
this.$parent = $parent;
} else {
return; // no parent - called too early?
}
render() {
// Modals are created at the time when `show()` is first called
if (!this.$modal) return;

const context = this.context;
const l10n = context.systems.l10n;
const scene = context.systems.gfx.scene;
const rtl = l10n.isRTL() ? '-rtl' : '';
const isRapidEnabled = scene.layers.get('rapid')?.enabled;

if (!this.$modal) return; // need to call `show()` first to create the modal.

const $content = this.$content;
const $content = this.$modal.select('.content');

/* Toggle All */
let $toggleAll = $content.selectAll('.rapid-toggle-all')
Expand Down Expand Up @@ -251,9 +241,9 @@ export class UiRapidDatasetToggle {
/**
* renderDatasets
* Renders the list of datasets into the `.rapid-datasets-container` div.
* @param {d3-selection} $container - A d3-selection to a HTMLElement that this component should render itself into
* @param {d3-selection} $selection - A d3-selection to a HTMLElement that this component should render itself into
*/
renderDatasets($container) {
renderDatasets($selection) {
const context = this.context;
const l10n = context.systems.l10n;
const map = context.systems.map;
Expand All @@ -266,7 +256,7 @@ export class UiRapidDatasetToggle {
const datasets = [...rapid.datasets.values()]
.filter(d => d.added && (showPreview || !d.beta)); // exclude preview datasets unless user has opted into them

let $rows = $container.selectAll('.rapid-checkbox-dataset')
let $rows = $selection.selectAll('.rapid-checkbox-dataset')
.data(datasets, d => d.id);

// exit
Expand Down Expand Up @@ -482,7 +472,7 @@ export class UiRapidDatasetToggle {

scene.dirtyLayers(['rapid', 'rapid-overlay', 'overture']);
gfx.immediateRedraw();
this.rerender();
this.render();

// In case a Rapid feature is already selected, reselect it to update sidebar too
const mode = context.mode;
Expand Down
Loading

0 comments on commit d0fec24

Please sign in to comment.