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

Hide 'Advanced' options by default. #6038

Merged
merged 1 commit into from Apr 9, 2018

Conversation

Projects
None yet
6 participants
@DannyCooper
Contributor

DannyCooper commented Apr 6, 2018

Description

Hide the 'Advanced' options in the Inspector by default.

By definition 'Advanced' options won't be used by most of the time so it makes sense to hide them behind a toggle.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.
@paulwilde

This comment has been minimized.

Contributor

paulwilde commented Apr 6, 2018

Agreed. Also I'm not sure whether it's been implemented (and would be separate from this PR anyway), but it'd also be nice if the open/closed state is saved per user.

@gziolo gziolo requested review from karmatosed and jasmussen Apr 6, 2018

@gziolo gziolo added the Blocks label Apr 6, 2018

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Apr 9, 2018

With this PR merged, the "Advanced" box at the bottom of this panel, and all block panels, will start off closed, correct?

screen shot 2018-04-09 at 08 50 34

If yes, then sure, I've no objection to this. I also have no strong opinions personally.

I do tend to agree with Paul Wilde that it would be nice, longer term, to store this configuration on a per-user basis, so if for example you often use the advanced section open for the Gallery block (for this, for example), then the open state for that panel is remembered. But that's outside the scope of this PR.

I'll let @karmatosed make the final call, and merge, if she agrees the advanced panel should start closed.

Also, thank you for the PR! 🏅

@DannyCooper

This comment has been minimized.

Contributor

DannyCooper commented Apr 9, 2018

Hi @jasmussen,

Yes that was the intended behaviour for this PR.

@gziolo

This comment has been minimized.

Member

gziolo commented Apr 9, 2018

Code wise is good to go, waiting for @karmatosed 👍

@gziolo

gziolo approved these changes Apr 9, 2018

@gziolo

This comment has been minimized.

Member

gziolo commented Apr 9, 2018

Related discussion in #6023, where @jasmussen proposes how to approach all inspector controls as a whole. In particular, this comment is relevant:

Decide a pattern for which panels are open by default. For example we could decide that only the topmost panel is open, all subsequent ones are closed. A future PR that stores this state on a user basis would then ameliorate any commonly used settings, while still keeping a strong default pattern.

@karmatosed

I don't see anything wrong with it being closed but also really like the idea it could be per user. My only concern is how discoverable the CSS class is. We can judge that over time. It is incredibly useful for a lot of people.

@gziolo gziolo merged commit 1dd320d into WordPress:master Apr 9, 2018

2 checks passed

codecov/project 44.51% remains the same compared to 0fa8d86
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gziolo

This comment has been minimized.

Member

gziolo commented Apr 9, 2018

Agreed. Also I'm not sure whether it's been implemented (and would be separate from this PR anyway), but it'd also be nice if the open/closed state is saved per user.

Don't we have a similar behavior implemented for Document tab? If yes, it might be not that difficult to make it more general and include it for blocks, too. My only concern is how many different combinations are there to cover. There is another question whether it should work per block type, or per individual block?

DannyCooper added a commit to DannyCooper/gutenberg that referenced this pull request Aug 21, 2018

Update CONTRIBUTORS.md
Had 2 (small) PRs committed so far:

WordPress#6038
WordPress#9204

danielbachhuber added a commit that referenced this pull request Aug 22, 2018

Update CONTRIBUTORS.md (#9217)
Had 2 (small) PRs committed so far:

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