[Feature] Multiple toolbox items: using of data overrides instead of config overrides#2064
Merged
neSpecc merged 4 commits intofeat/vertical-toolbox-multiple-itemsfrom May 18, 2022
Conversation
neSpecc
requested review from
TatianaFomina,
gohabereg and
ilyamore88
as code owners
Contributor
TatianaFomina
left a comment
TatianaFomina
left a comment
There was a problem hiding this comment.
Got several comments but other than that looks like a big improvement
src/components/block/index.ts
Outdated
| return toolboxItems.find((item) => { | ||
| return Object.entries(item.data) | ||
| .some(([propName, propValue]) => { | ||
| return blockData[propName] && blockData[propName] === propValue; |
Contributor
There was a problem hiding this comment.
can data have nested objects or arrays? or it's always flat?
| * Get icon or title for dropdown | ||
| */ | ||
| const toolboxSettings = currentBlock.activeToolboxEntry || {}; | ||
| const toolboxSettings = await currentBlock.getActiveToolboxEntry() || {}; |
Contributor
There was a problem hiding this comment.
there's one problem with setting toggler's content this way: if none of the configured toolbox entries is active, toggler will contain only tool name
Screen.Recording.2022-05-18.at.09.46.01.mov
Member
Author
There was a problem hiding this comment.
I think, it is ok because other titles and icons does not correctly fit here.
TatianaFomina
approved these changes
May 18, 2022
TatianaFomina
added a commit
that referenced
this pull request
Jun 17, 2022
* the popover component, vertical toolbox * toolbox position improved * popover width improved * always show the plus button * search field added * search input in popover * trying to create mobile toolbox * FIx mobile popover fixed positioning * Add mobile popover overlay * Hide mobile popover on scroll * Tmp * feat(toolbox): popover adapted for mobile devices (#2004) * FIx mobile popover fixed positioning * Add mobile popover overlay * Hide mobile popover on scroll * Alter toolbox buttons hover * Fix closing popover on overlay click * Tests fix * Fix onchange test * restore focus after toolbox closing by ESC * don't move toolbar by block-hover on mobile Resolves #1972 * popover mobile styles improved * Cleanup * Remove scroll event listener * Lock scroll on mobile * don't show shortcuts in mobile popover * Change data attr name * Remove unused styles * Remove unused listeners * disable hover on mobile popover * Scroll fix * Lint * Revert "Scroll fix" This reverts commit 82deae5. * Return back background color for active state of toolbox buttons Co-authored-by: Peter Savchenko <specc.dev@gmail.com> * Vertical toolbox fixes (#2017) * Replace visibility property with display for hiding popover * Disable arrow right and left keys for popover * Revert "Replace visibility property with display for hiding popover" This reverts commit af521cf. * Hide popover via setting max-height to 0 to fix animation in safari * Remove redundant condition * Extend element interface to avoid ts errors * Do not subscribe to block hovered if mobile * Add unsubscribing from overlay click event * Rename isMobile to isMobileScreen * Cleanup * fix: popover opening direction (#2022) * Change popover opening direction based on available space below it * Update check * Use cacheable decorator * Update src/components/flipper.ts Co-authored-by: George Berezhnoy <gohabereg@users.noreply.github.com> * Fixes * Fix test * Clear search on popover hide * Fix popover width * Fix for tests * Update todos * Linter fixes * rm todo about beforeInsert because I have no idea what does it mean * i18n for search labels done * rm methods for hiding/showing of + * some code style update * Update CHANGELOG.md * make the list items a little bit compact * fix z-index issue caused by block-appearing animation also, improve popover padding for two reasons: - make the popover more consistent with the Table tool popover (in future, it can be done with the same api method) - make popover looks better * Some progress Use overriden config tmp * Cleanup * Proceed cleanup * Update tool-settings.d.ts * Get rid of isToolboxItemActive * Get rid of key * Filter out duplicates in conversion menu * Rename hash to id * Change function for generating hash * Cleanup * Further cleanup * [Feature] Multiple toolbox items: using of data overrides instead of config overrides (#2064) * Use data instead of config * check if active toolbox entry exists * comparison improved * eslint fix * rename toolbox types, simplify hasTools method * add empty line * wrong line * add multiple toobox note to the doc * Update toolbox configs merge logic * Add a test case * Add toolbox ui tests * Update tests * upd doc * Update header * Update changelog and package.json * Update changelog * Update jsdoc * Remove unused dependency * Make BlockTool's toolbox getter always return an array * Fix for unconfigured toolbox * Revert "Fix for unconfigured toolbox" This reverts commit dff1df2. * Change return type * Merge data overrides with actual block data when inserting a block * Revert "Merge data overrides with actual block data when inserting a block" This reverts commit eb0a59c. * Merge tool's data with data overrides * Move merging block data with data overrides to insertNewBlock * Update changelog * Rename getDefaultBlockData to composeBlockData * Create block data on condition * Update types/api/blocks.d.ts Co-authored-by: Peter Savchenko <specc.dev@gmail.com> * Update src/components/modules/api/blocks.ts Co-authored-by: Peter Savchenko <specc.dev@gmail.com> Co-authored-by: Peter Savchenko <specc.dev@gmail.com> Co-authored-by: George Berezhnoy <gohabereg@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a proposal change to the #2050
See also changes on the Block API side: editor-js/header#80
I'm suggesting to use the
dataoverrides instead of theconfigoverridesIt allows:
activeToolboxEntrygetter (it could be matched by current block data)configthrough many methods