Skip to content

feat: nested popover #2649

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

Merged
merged 39 commits into from
Apr 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
48247e8
Move popover types to separate file
TatianaFomina Feb 3, 2024
57242bf
tmp
TatianaFomina Feb 4, 2024
2609ac5
open top
TatianaFomina Feb 18, 2024
1c7925d
Fix bug with keyboard navigation
TatianaFomina Mar 9, 2024
86142f9
Fix bug with scroll
TatianaFomina Mar 9, 2024
6be9cf2
Fix mobile
TatianaFomina Mar 9, 2024
44a63ac
Add popover header class
TatianaFomina Mar 9, 2024
77d0dcd
Display nested items on mobile
TatianaFomina Mar 9, 2024
04d2948
Refactor history
TatianaFomina Mar 13, 2024
2f9d0fb
Fix positioning on desktop
TatianaFomina Mar 16, 2024
9821a3f
Fix tests
TatianaFomina Mar 16, 2024
8e65390
Fix child popover indent left
TatianaFomina Mar 16, 2024
cef51c7
Fix ts errors in popover files
TatianaFomina Mar 16, 2024
6871c0a
Move files
TatianaFomina Mar 16, 2024
de7c00e
Rename cn to bem
TatianaFomina Mar 23, 2024
d19665c
Clarify comments and rename method
TatianaFomina Mar 23, 2024
6502e71
Refactor popover css classes
TatianaFomina Mar 23, 2024
6dd5141
Rename cls to css
TatianaFomina Mar 23, 2024
1c2dc42
Split popover desktop and mobile classes
TatianaFomina Mar 27, 2024
cf222e8
Add ability to open popover to the left if not enough space to open t…
TatianaFomina Mar 30, 2024
b761e4a
Add nested popover test
TatianaFomina Mar 23, 2024
f36cb86
Add popover test for mobile screens
TatianaFomina Mar 23, 2024
da00f16
Fix tests
TatianaFomina Mar 30, 2024
fe4cdf6
Add union type for both popovers
TatianaFomina Mar 30, 2024
b056297
Merge branch 'next' into feat/nested-popover
TatianaFomina Apr 3, 2024
ce35b59
Add global window resize event
TatianaFomina Apr 3, 2024
75d8476
Multiple fixes
TatianaFomina Apr 3, 2024
7fab576
Move nodes initialization to constructor
TatianaFomina Apr 6, 2024
0bce02c
Rename handleShowingNestedItems to showNestedItems
TatianaFomina Apr 6, 2024
b13f8c0
Replace WindowResize with EditorMobileLayoutToggled
TatianaFomina Apr 6, 2024
dcdb8fb
New doze of fixes
TatianaFomina Apr 7, 2024
c2a7c8f
Review fixes
TatianaFomina Apr 8, 2024
acd2710
Fixes
TatianaFomina Apr 8, 2024
b5fbccb
Fixes
TatianaFomina Apr 8, 2024
325aeac
Make each nested popover decide itself if it should open top
TatianaFomina Apr 13, 2024
5942b18
Update changelog
TatianaFomina Apr 13, 2024
7611e0a
Update changelog
TatianaFomina Apr 13, 2024
03aa22c
Update changelog
TatianaFomina Apr 13, 2024
2d9d73a
Merge branch 'next' into feat/nested-popover
TatianaFomina Apr 13, 2024
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
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### 2.30.1

– `New` – Block Tunes now supports nesting items

### 2.30.0

- `Improvement` — The ability to merge blocks of different types (if both tools provide the conversionConfig)
Expand Down
6 changes: 4 additions & 2 deletions src/components/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ export default class Dom {
* @param {object} [attributes] - any attributes
* @returns {HTMLElement}
*/
public static make(tagName: string, classNames: string | string[] | null = null, attributes: object = {}): HTMLElement {
public static make(tagName: string, classNames: string | (string | undefined)[] | null = null, attributes: object = {}): HTMLElement {
const el = document.createElement(tagName);

if (Array.isArray(classNames)) {
el.classList.add(...classNames);
const validClassnames = classNames.filter(className => className !== undefined) as string[];

el.classList.add(...validClassnames);
} else if (classNames) {
el.classList.add(classNames);
}
Expand Down
15 changes: 15 additions & 0 deletions src/components/events/EditorMobileLayoutToggled.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Fired when editor mobile layout toggled
*/
export const EditorMobileLayoutToggled = 'editor mobile layout toggled';

/**
* Payload that will be passed with the event
*/
export interface EditorMobileLayoutToggledPayload {
/**
* True, if mobile layout enabled
*/
isEnabled: boolean;
}

5 changes: 4 additions & 1 deletion src/components/events/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { BlockChanged, BlockChangedPayload } from './BlockChanged';
import { BlockHovered, BlockHoveredPayload } from './BlockHovered';
import { FakeCursorAboutToBeToggled, FakeCursorAboutToBeToggledPayload } from './FakeCursorAboutToBeToggled';
import { FakeCursorHaveBeenSet, FakeCursorHaveBeenSetPayload } from './FakeCursorHaveBeenSet';
import { EditorMobileLayoutToggled, EditorMobileLayoutToggledPayload } from './EditorMobileLayoutToggled';

/**
* Events fired by Editor Event Dispatcher
Expand All @@ -11,7 +12,8 @@ export {
RedactorDomChanged,
BlockChanged,
FakeCursorAboutToBeToggled,
FakeCursorHaveBeenSet
FakeCursorHaveBeenSet,
EditorMobileLayoutToggled
};

/**
Expand All @@ -23,4 +25,5 @@ export interface EditorEventMap {
[BlockChanged]: BlockChangedPayload;
[FakeCursorAboutToBeToggled]: FakeCursorAboutToBeToggledPayload;
[FakeCursorHaveBeenSet]: FakeCursorHaveBeenSetPayload;
[EditorMobileLayoutToggled]: EditorMobileLayoutToggledPayload
}
9 changes: 2 additions & 7 deletions src/components/flipper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,11 @@ export default class Flipper {

/**
* Instance of flipper iterator
*
* @type {DomIterator|null}
*/
private readonly iterator: DomIterator = null;
private readonly iterator: DomIterator | null = null;

/**
* Flag that defines activation status
*
* @type {boolean}
*/
private activated = false;

Expand All @@ -77,7 +73,7 @@ export default class Flipper {
private flipCallbacks: Array<() => void> = [];

/**
* @param {FlipperOptions} options - different constructing settings
* @param options - different constructing settings
*/
constructor(options: FlipperOptions) {
this.iterator = new DomIterator(options.items, options.focusedItemClass);
Expand Down Expand Up @@ -110,7 +106,6 @@ export default class Flipper {
*/
public activate(items?: HTMLElement[], cursorPosition?: number): void {
this.activated = true;

if (items) {
this.iterator.setItems(items);
}
Expand Down
36 changes: 24 additions & 12 deletions src/components/modules/toolbar/blockSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { I18nInternalNS } from '../../i18n/namespace-internal';
import Flipper from '../../flipper';
import { TunesMenuConfigItem } from '../../../../types/tools';
import { resolveAliases } from '../../utils/resolve-aliases';
import Popover, { PopoverEvent } from '../../utils/popover';
import { type Popover, PopoverDesktop, PopoverMobile } from '../../utils/popover';
import { PopoverEvent } from '../../utils/popover/popover.types';
import { isMobileScreen } from '../../utils';
import { EditorMobileLayoutToggled } from '../../events';

/**
* HTML Elements that used for BlockSettings
Expand All @@ -27,8 +30,6 @@ interface BlockSettingsNodes {
export default class BlockSettings extends Module<BlockSettingsNodes> {
/**
* Module Events
*
* @returns {{opened: string, closed: string}}
*/
public get events(): { opened: string; closed: string } {
return {
Expand Down Expand Up @@ -56,8 +57,12 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {
*
* @todo remove once BlockSettings becomes standalone non-module class
*/
public get flipper(): Flipper {
return this.popover?.flipper;
public get flipper(): Flipper | undefined {
if (this.popover === null) {
return;
}

return 'flipper' in this.popover ? this.popover?.flipper : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

this.popover?.flipper should work the same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah, there will be TypeScript error. Property flipper exists only in PopoverDesktop and not in PopoverMobile, that's why we need to check weather the property is present

}

/**
Expand All @@ -67,9 +72,9 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {

/**
* Popover instance. There is a util for vertical lists.
* Null until popover is not initialized
*/
private popover: Popover | undefined;

private popover: Popover | null = null;

/**
* Panel with block settings with 2 sections:
Expand All @@ -82,13 +87,17 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {
if (import.meta.env.MODE === 'test') {
this.nodes.wrapper.setAttribute('data-cy', 'block-tunes');
}

this.eventsDispatcher.on(EditorMobileLayoutToggled, this.close);
}

/**
* Destroys module
*/
public destroy(): void {
this.removeAllNodes();
this.listeners.destroy();
this.eventsDispatcher.off(EditorMobileLayoutToggled, this.close);
}

/**
Expand Down Expand Up @@ -118,7 +127,10 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {

/** Tell to subscribers that block settings is opened */
this.eventsDispatcher.emit(this.events.opened);
this.popover = new Popover({

const PopoverClass = isMobileScreen() ? PopoverMobile : PopoverDesktop;

this.popover = new PopoverClass({
searchable: true,
items: tunesItems.map(tune => this.resolveTuneAliases(tune)),
customContent: customHtmlTunesContainer,
Expand All @@ -132,22 +144,22 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {

this.popover.on(PopoverEvent.Close, this.onPopoverClose);

this.nodes.wrapper.append(this.popover.getElement());
this.nodes.wrapper?.append(this.popover.getElement());

this.popover.show();
}

/**
* Returns root block settings element
*/
public getElement(): HTMLElement {
public getElement(): HTMLElement | undefined {
return this.nodes.wrapper;
}

/**
* Close Block Settings pane
*/
public close(): void {
public close = (): void => {
if (!this.opened) {
return;
}
Expand Down Expand Up @@ -183,7 +195,7 @@ export default class BlockSettings extends Module<BlockSettingsNodes> {
this.popover.getElement().remove();
this.popover = null;
}
}
};

/**
* Handles popover close event
Expand Down
4 changes: 3 additions & 1 deletion src/components/modules/toolbar/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ export default class Toolbar extends Module<ToolbarNodes> {
};
}


/**
* Toggles read-only mode
*
Expand Down Expand Up @@ -479,9 +480,10 @@ export default class Toolbar extends Module<ToolbarNodes> {
}
});

return this.toolboxInstance.make();
return this.toolboxInstance.getElement();
}


/**
* Handler for Plus Button
*/
Expand Down
22 changes: 17 additions & 5 deletions src/components/modules/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { mobileScreenBreakpoint } from '../utils';
import styles from '../../styles/main.css?inline';
import { BlockHovered } from '../events/BlockHovered';
import { selectionChangeDebounceTimeout } from '../constants';
import { EditorMobileLayoutToggled } from '../events';
/**
* HTML Elements used for UI
*/
Expand Down Expand Up @@ -121,7 +122,7 @@ export default class UI extends Module<UINodes> {
/**
* Detect mobile version
*/
this.checkIsMobile();
this.setIsMobile();

/**
* Make main UI elements
Expand Down Expand Up @@ -234,10 +235,21 @@ export default class UI extends Module<UINodes> {
}

/**
* Check for mobile mode and cache a result
* Check for mobile mode and save the result
*/
private checkIsMobile(): void {
this.isMobile = window.innerWidth < mobileScreenBreakpoint;
private setIsMobile(): void {
const isMobile = window.innerWidth < mobileScreenBreakpoint;

if (isMobile !== this.isMobile) {
/**
* Dispatch global event
*/
this.eventsDispatcher.emit(EditorMobileLayoutToggled, {
isEnabled: this.isMobile,
});
}

this.isMobile = isMobile;
}

/**
Expand Down Expand Up @@ -426,7 +438,7 @@ export default class UI extends Module<UINodes> {
/**
* Detect mobile version
*/
this.checkIsMobile();
this.setIsMobile();
}

/**
Expand Down
Loading