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

Chrome: Use expandable panels for the block inspector #2820

Merged
merged 3 commits into from Oct 2, 2017

Conversation

Projects
None yet
4 participants
@youknowriad
Contributor

youknowriad commented Sep 28, 2017

Description

fixes #2769 This PR adds expandable panels to the block inspector for the text block. we'll have to think block by block how to group things together.

Screenshots

screen shot 2017-09-28 at 11 14 37

Question:

  • I ended up overriding the PanelBody styling for the inspector panels, I wonder if it's worth creating a separate component or if we're ok with these overrides.

  • The open/closed state is not persisted. If we want to do so, it will create a lot of overhead because we need per block persistence and we do not want to introduce a block API just for this. ( we can change the default opened state value of each panel separately)

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Sep 28, 2017

Codecov Report

Merging #2820 into master will increase coverage by 0.13%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2820      +/-   ##
==========================================
+ Coverage    33.8%   33.94%   +0.13%     
==========================================
  Files         190      191       +1     
  Lines        5680     6202     +522     
  Branches      992     1136     +144     
==========================================
+ Hits         1920     2105     +185     
- Misses       3183     3424     +241     
- Partials      577      673      +96
Impacted Files Coverage Δ
editor/sidebar/block-inspector/index.js 0% <ø> (ø) ⬆️
blocks/library/paragraph/index.js 33.33% <0%> (ø) ⬆️
components/panel/body.js 92.3% <100%> (ø) ⬆️
blocks/api/paste/utils.js 88.88% <0%> (-8.62%) ⬇️
blocks/api/paste/index.js 84.09% <0%> (-3.91%) ⬇️
blocks/library/code/index.js 53.84% <0%> (-3.3%) ⬇️
blocks/editable/index.js 8.92% <0%> (-1.4%) ⬇️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️
blocks/library/preformatted/index.js 50% <0%> (ø) ⬆️
blocks/color-palette/index.js 0% <0%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb5f014...6fcd749. Read the comment docs.

codecov bot commented Sep 28, 2017

Codecov Report

Merging #2820 into master will increase coverage by 0.13%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2820      +/-   ##
==========================================
+ Coverage    33.8%   33.94%   +0.13%     
==========================================
  Files         190      191       +1     
  Lines        5680     6202     +522     
  Branches      992     1136     +144     
==========================================
+ Hits         1920     2105     +185     
- Misses       3183     3424     +241     
- Partials      577      673      +96
Impacted Files Coverage Δ
editor/sidebar/block-inspector/index.js 0% <ø> (ø) ⬆️
blocks/library/paragraph/index.js 33.33% <0%> (ø) ⬆️
components/panel/body.js 92.3% <100%> (ø) ⬆️
blocks/api/paste/utils.js 88.88% <0%> (-8.62%) ⬇️
blocks/api/paste/index.js 84.09% <0%> (-3.91%) ⬇️
blocks/library/code/index.js 53.84% <0%> (-3.3%) ⬇️
blocks/editable/index.js 8.92% <0%> (-1.4%) ⬇️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️
blocks/library/preformatted/index.js 50% <0%> (ø) ⬆️
blocks/color-palette/index.js 0% <0%> (ø) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb5f014...6fcd749. Read the comment docs.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

Love it, works great!

I'm seeing some CSS issues, but I think they are due to me having some compilation issues and docker issues, not quite sure what it is. Anyone else seeing this?

screen shot 2017-09-28 at 13 02 44

Contributor

jasmussen commented Sep 28, 2017

Love it, works great!

I'm seeing some CSS issues, but I think they are due to me having some compilation issues and docker issues, not quite sure what it is. Anyone else seeing this?

screen shot 2017-09-28 at 13 02 44

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

(Provided that CSS stuff is specific to my computer and not this branch, then 👍 👍 from me)

Contributor

jasmussen commented Sep 28, 2017

(Provided that CSS stuff is specific to my computer and not this branch, then 👍 👍 from me)

@youknowriad youknowriad requested a review from mcsf Sep 29, 2017

@mcsf

This comment has been minimized.

Show comment
Hide comment
@mcsf

mcsf Sep 29, 2017

Contributor

I'm seeing some CSS issues

I am getting these too. Haven't debugged yet.

Contributor

mcsf commented Sep 29, 2017

I'm seeing some CSS issues

I am getting these too. Haven't debugged yet.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Sep 29, 2017

Contributor

@mcsf It looks like this was caused by a wrong way of importing the UI components

import { PanelBody } from 'components';

replaced by

import { PanelBody } from '@wordpress/components';

The first one causes the components module to be embedded in the blocks module, Not sure why this was causing the style issues though.

Anyway it should be better now

Contributor

youknowriad commented Sep 29, 2017

@mcsf It looks like this was caused by a wrong way of importing the UI components

import { PanelBody } from 'components';

replaced by

import { PanelBody } from '@wordpress/components';

The first one causes the components module to be embedded in the blocks module, Not sure why this was causing the style issues though.

Anyway it should be better now

@mcsf

mcsf approved these changes Sep 29, 2017

The implications of mis-importing components are fascinating. :)

Can confirm this works well now.

@youknowriad youknowriad merged commit 3b64ebc into master Oct 2, 2017

3 checks passed

codecov/project 33.94% (+0.13%) compared to eb5f014
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@youknowriad youknowriad deleted the update/inspector-panels branch Oct 2, 2017

@mtias

This comment has been minimized.

Show comment
Hide comment
@mtias

mtias Oct 2, 2017

Contributor

@youknowriad the overwrite seems fine to me. We are going to need issues for the remaining blocks.

Contributor

mtias commented Oct 2, 2017

@youknowriad the overwrite seems fine to me. We are going to need issues for the remaining blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment