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.

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

Fix inconsistent references to Settings Sidebar #16138

Merged
merged 2 commits into from Jul 5, 2019

Conversation

@sarahmonster
Copy link
Member

commented Jun 12, 2019

This swaps out instances of Inspector and Block Settings with Settings Sidebar, as per our Design Patterns doc. I think I've got them all, but would appreciate a double-check.

I've also corrected a few instances of unclear references to the Block Toolbar when I came across them, and I've updated the code where it made sense and wouldn't break things, so we are more consistent about how we refer to things. The term "Inspector" still appears as the name of a React component, but that's a bigger change I'd rather not initiate here.

I've also changed a filename and slug, which I'm hoping won't break things, but we can revert if those will cause issues.

Fixes #14988.

@kjellr
kjellr approved these changes Jun 12, 2019
Copy link
Contributor

left a comment

Copy-wise, this is in great shape and gets a from me.

Before merge though, I'd like to get one final look from someone familiar with renaming files though to make sure this will translate correctly to the WP.org mirror. @gziolo can you take a look if you have a moment?

Thanks!

@gziolo

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Before merge though, I'd like to get one final look from someone familiar with renaming files though to make sure this will translate correctly to the WP.org mirror. @gziolo can you take a look if you have a moment?

@chrisvanpatten or @nosolosw should know better, I have no idea how it works at the moment. The biggest concern I would personally have is that we should start adding redirects to every URL we change to ensure they work when linked from external websites.

@sarahmonster

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Lovely, I wasn't sure about changing the URL slugs and whether or not they'd redirect. I can roll them back if it seems like it'd be problematic, but if they will redirect it'd be best to change these references in as many places as possible, to reinforce consistent terminology.

@nosolosw

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

👋 There are a couple of other places in the docs where we are using inspector rather than settings sidebar: glossary, block registration.

I've also noticed that the code primitives refer to Inspector (see). Probably that was the term used since the beginning (see glossary). I'd like both code and docs to refer to the same terms, as to have a common vocabulary among all of us, and also not to confuse authors that want to use the block editor APIs. Changing the code is feasible, but I also understand it'll be a slightly bigger challenge that's perhaps best addressed in a different PR.

URLs

One thing that we can do to know whether an URL will redirect automatically is to substitute the old slug by the new one. For example:

It won't work in this case. It's possible to ask people in meta to add this redirect, though. If we wanted to change the URL, we should probably add the redirects it as to not break existing external references to the handbook.

@sarahmonster

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

I'd like both code and docs to refer to the same terms, as to have a common vocabulary among all of us

Agreed x 1,000,000!

I noticed this in the code but wasn't comfortable changing it myself for what are probably obvious reasons. 😄 I'm happy to file a follow-up issue though.

Good to know that the URL wouldn't automatically update. Do you think we should just keep the existing URL, or ask for a redirect to be made manually? I'm definitely inclined to change the references in all instances (text, code, and URLs) for better consistency across all areas of the project, but I understand if this could prove more work than strictly warranted.

…debar; fixes #14988.
@mkaz mkaz force-pushed the fix/inconsistent-sidebar-terminology-14988 branch from 1cb0de1 to d80325b Jul 5, 2019
@mkaz mkaz merged commit 707336c into master Jul 5, 2019
1 of 2 checks passed
1 of 2 checks passed
Filter merged Filter merged
Details
Travis CI - Pull Request Build Passed
Details
@github-actions github-actions bot added this to the Gutenberg 6.1 milestone Jul 5, 2019
@mkaz

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

I've merged and closing this one out to move forward with the changes.

I've created an issue to address code here: #16437

I created this Meta Trac ticket for redirect: https://meta.trac.wordpress.org/ticket/4582#ticket

And will open a new PR and reference this one for additional documentation changes.

@mkaz mkaz deleted the fix/inconsistent-sidebar-terminology-14988 branch Jul 5, 2019
daniloercoli added a commit that referenced this pull request Jul 8, 2019
…rnmobile/track-unsupported-blocks

* 'master' of https://github.com/WordPress/gutenberg:
  Bump plugin version to 6.1.0-rc.1
  Update HTML anchor explaination text (#16142)
  Move post permalink to beneath title on mobile. (#16277)
  Export cloneBlock method to the mobile (#16441)
  Fix inconsistent references to Settings Sidebar (#16138)
  Adds a cache key to the blocks reducer in order to optimize the getBlock selector (#16407)
  Track the parent block to optimize hierarchy selectors (#16392)
Tug added a commit that referenced this pull request Jul 9, 2019
* Switch out instances of Inspector and Block Settings with Settings Sidebar; fixes #14988.

* Results for npm run docs:build (whitespace)
jg314 added a commit to jg314/gutenberg that referenced this pull request Jul 19, 2019
* Switch out instances of Inspector and Block Settings with Settings Sidebar; fixes WordPress#14988.

* Results for npm run docs:build (whitespace)
sbardian added a commit to sbardian/gutenberg that referenced this pull request Jul 29, 2019
* Switch out instances of Inspector and Block Settings with Settings Sidebar; fixes WordPress#14988.

* Results for npm run docs:build (whitespace)
dd32 pushed a commit to dd32/gutenberg that referenced this pull request Sep 27, 2019
* Switch out instances of Inspector and Block Settings with Settings Sidebar; fixes WordPress#14988.

* Results for npm run docs:build (whitespace)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.