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.

Sign up for GitHub

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

List View: Reduce whitespace and always show nested blocks #28029

Merged
merged 9 commits into from
Jan 13, 2021
Merged

List View: Reduce whitespace and always show nested blocks #28029

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c162de7
Forcing nested blocks to always show, but there's gotta be a better way.
shaunandrews Jan 6, 2021
1f0f1c9
Reducing whitespace and simplifying the descenders.
shaunandrews Jan 6, 2021
55518ef
Adjusting the selected state.
shaunandrews Jan 6, 2021
67367e5
Moving the hasHierarchy check into the value of blocks.
shaunandrews Jan 7, 2021
f25c475
This isn't needed. Leftover from some earlier explorations.
shaunandrews Jan 7, 2021
52f5c0b
Overriding the hover state when selected.
shaunandrews Jan 7, 2021
eba63e7
Simplifying the descener-lines component and moving it inside the but…
shaunandrews Jan 8, 2021
fd814c4
Merge branch 'master' into try/list-view-tweaks
shaunandrews Jan 11, 2021
02c782f
Renaming the descender-lines component to indentation.
shaunandrews Jan 13, 2021
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
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/block-navigation/block-select-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { __ } from '@wordpress/i18n';
* Internal dependencies
*/
import BlockIcon from '../block-icon';
import DescenderLines from './descender-lines';
import useBlockDisplayInformation from '../use-block-display-information';
import { getBlockPositionDescription } from './utils';

Expand Down Expand Up @@ -70,6 +71,7 @@ function BlockNavigationBlockSelectButton(
onDragEnd={ onDragEnd }
draggable={ draggable }
>
<DescenderLines level={ level } />
<BlockIcon icon={ blockInformation?.icon } showColors />
{ blockDisplayName }
{ isSelected && (
Expand Down
7 changes: 0 additions & 7 deletions packages/block-editor/src/components/block-navigation/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
BlockMoverUpButton,
BlockMoverDownButton,
} from '../block-mover/button';
import DescenderLines from './descender-lines';
import BlockNavigationBlockContents from './block-contents';
import BlockSettingsDropdown from '../block-settings-menu/block-settings-dropdown';
import { useBlockNavigationContext } from './context';
Expand All @@ -39,7 +38,6 @@ export default function BlockNavigationBlock( {
rowCount,
siblingBlockCount,
showBlockMovers,
terminatedLevels,
path,
} ) {
const cellRef = useRef( null );
Expand Down Expand Up @@ -108,11 +106,6 @@ export default function BlockNavigationBlock( {
>
{ ( { ref, tabIndex, onFocus } ) => (
<div className="block-editor-block-navigation-block__contents-container">
<DescenderLines
level={ level }
isLastRow={ position === rowCount }
terminatedLevels={ terminatedLevels }
/>
<BlockNavigationBlockContents
block={ block }
onClick={ () => onClick( block.clientId ) }
Expand Down
8 changes: 1 addition & 7 deletions packages/block-editor/src/components/block-navigation/descender-lines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ import classnames from 'classnames';

const lineClassName = 'block-editor-block-navigator-descender-line';

export default function DescenderLines( {
level,
isLastRow,
terminatedLevels,
} ) {
export default function DescenderLines( { level } ) {
shaunandrews marked this conversation as resolved.
Show resolved Hide resolved
return times( level - 1, ( index ) => {
// The first 'level' that has a descender line is level 2.
// Add 2 to the zero-based index below to reflect that.
Expand All @@ -22,8 +18,6 @@ export default function DescenderLines( {
aria-hidden="true"
className={ classnames( lineClassName, {
'has-item': hasItem,
'is-last-row': isLastRow,
'is-terminated': terminatedLevels.includes( currentLevel ),
} ) }
/>
);
Expand Down
25 changes: 8 additions & 17 deletions packages/block-editor/src/components/block-navigation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,14 @@ function BlockNavigation( {
<p className="block-editor-block-navigation__label">
{ __( 'List view' ) }
</p>
{ hasHierarchy && (
<BlockNavigationTree
blocks={ [ rootBlock ] }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
__experimentalFeatures={ __experimentalFeatures }
showNestedBlocks
/>
) }
{ ! hasHierarchy && (
<BlockNavigationTree
blocks={ rootBlocks }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
__experimentalFeatures={ __experimentalFeatures }
/>
) }

<BlockNavigationTree
blocks={ hasHierarchy ? [ rootBlock ] : rootBlocks }
selectedBlockClientId={ selectedBlockClientId }
selectBlock={ selectBlock }
__experimentalFeatures={ __experimentalFeatures }
showNestedBlocks
/>
Comment on lines +39 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

The hasHierarchy ? [ rootBlock ] : rootBlocks bit and the associated code in the withSelect function lower down in this file means that when a child block is selected, ListView zooms in to only show a limited part of the tree.

So that could be changed as well. I wonder if it might be good to make the changes to ListView's logic a separate PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't quite wrap my brain around this logic — its very strange to me, but its the existing behavior. I'm happy to change it, but it might make more sense in a separate PR, perhaps along with adding a sidebar for this list view.

</div>
);
}
Expand Down
93 changes: 20 additions & 73 deletions packages/block-editor/src/components/block-navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
$tree-border-width: 2px;
$tree-item-height: 36px;

.block-editor-block-navigation__label {
margin: 0 0 $grid-unit-15;
color: $gray-700;
Expand All @@ -10,7 +7,7 @@ $tree-item-height: 36px;
}

.block-editor-block-navigation__container {
padding: $grid-unit - $border-width;
min-width: 280px;
}

.block-editor-block-navigation-tree {
Expand All @@ -24,6 +21,13 @@ $tree-item-height: 36px;
// Use position relative for row animation.
position: relative;

&.is-selected .block-editor-block-navigation-block-contents,
&.is-selected:hover .block-editor-block-navigation-block-contents,
&.is-selected:focus .block-editor-block-navigation-block-contents {
background: $gray-900;
color: $white;
}

&.is-dragging {
display: none;
}
Expand All @@ -33,15 +37,21 @@ $tree-item-height: 36px;
align-items: center;
width: 100%;
height: auto;
padding: $grid-unit-15 ($grid-unit-15 / 2);
margin-top: auto;
margin-bottom: auto;
padding: ($grid-unit-15 / 2);
text-align: left;
color: $gray-900;
border-radius: 2px;
position: relative;
white-space: nowrap;

&:hover {
background: $gray-100;
}

&:focus {
z-index: 1;
}

&.is-dropping-before::before {
content: "";
position: absolute;
Expand Down Expand Up @@ -91,14 +101,6 @@ $tree-item-height: 36px;
margin-right: 6px;
}

&.is-selected .block-editor-block-icon svg,
&.is-selected:focus .block-editor-block-icon svg {
color: $white;
background: $gray-900;
box-shadow: 0 0 0 $border-width $gray-900;
border-radius: $border-width;
}

.block-editor-block-navigation-block__menu-cell,
.block-editor-block-navigation-block__mover-cell,
.block-editor-block-navigation-block__contents-cell {
Expand Down Expand Up @@ -225,64 +227,9 @@ $tree-item-height: 36px;
.block-editor-block-navigation-appender__container {
display: flex;
}

.block-editor-block-navigation-block__contents-container {
min-height: $grid-unit-60;
}

.block-editor-block-navigator-descender-line {
position: relative;
flex-shrink: 0;
width: ( $button-size / 2 ) + 6px;

&:first-child {
width: ( $button-size / 2 );
}

&.has-item {
margin-right: 6px;
}

// Draw a vertical line using border-right.
&::before {
content: "";
display: block;
position: absolute;
top: 1px;
bottom: -2px;
right: -1px;
border-right: 2px solid $gray-600;
}

// If a vertical line has terminated, don't draw it.
&.is-terminated::before {
border-color: transparent;
}

// Make the last vertical line half-height.
&.has-item.is-last-row {
height: 26px;
}

// Draw a horizontal line using border-bottom.
&.has-item::after {
content: "";
display: block;
position: absolute;
top: 26px;
left: 100%;
width: 5px;
border-bottom: 2px solid $gray-600;
}
}
}

// Position the horizontal line in the middle of appender's height
.block-editor-block-navigation-appender__cell .block-editor-block-navigator-descender-line {
&.has-item.is-last-row {
height: $grid-unit-20;
&::after {
top: 100%;
}
}
.block-editor-block-navigator-descender-line {
flex-shrink: 0;
width: 18px;
}