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

Remove the inserter title from mobile inserters #14493

Merged
merged 2 commits into from Mar 21, 2019

Conversation

youknowriad
Copy link
Contributor

Related #14043

On mobile, we open the inserter (all inserters, globals, in-between...) we should the title of the current post at the top but in a generic block editor world, this title doesn't make sense as we could imagine multiple inserters (widgets areas) on the same page without a title to show...

While we can provide a "config" somehow to show a custom title for each block editor, I wonder if we could avoid this complexity and just remove the title or find a generic title to show there "Add Block" or something in that vein.

@youknowriad youknowriad self-assigned this Mar 18, 2019
@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Mar 18, 2019
@youknowriad youknowriad requested review from jasmussen and a team March 19, 2019 08:37
@jasmussen
Copy link
Contributor

Showing the title in the inserter did always confuse me a little bit, so in principle and with this new context, this makes a lot of sense to me personally.

I do recall the title being added at some point, I can't recall the reason, but leaving the "design feedback" label in case anyone recalls. Otherwise 👍 👍 from me.

@kjellr
Copy link
Contributor

kjellr commented Mar 19, 2019

Showing the title in the inserter did always confuse me a little bit, so in principle and with this new context, this makes a lot of sense to me personally.

+1, this makes a lot of sense to me too.

@@ -92,7 +92,6 @@ class Inserter extends Component {
position={ position }
onToggle={ this.onToggle }
expandOnMobile
headerTitle={ title }
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, this was our only usage of headerTitle. It's now dead code in Inserter. That said, there's a compatibility question of removing it, and I suppose there may be valid use-cases for it in the future.

@mapk
Copy link
Contributor

mapk commented Mar 19, 2019

Good call, @youknowriad.

I wonder if we could avoid this complexity and just remove the title or find a generic title to show there "Add Block" or something in that vein.

Can we just call it "Block Inserter" or "Inserter" or @karmatosed's favorite, "Block Library"? I ask b/c this might be a good place to reinforce terminology with WP users.

@kjellr
Copy link
Contributor

kjellr commented Mar 20, 2019

Can we just call it "Block Inserter" or "Inserter" or @karmatosed's favorite, "Block Library"? I ask b/c this might be a good place to reinforce terminology with WP users.

That's an interesting thought. This could also act as a sort of label: "Add a block"

@jasmussen
Copy link
Contributor

It's worth mentioning just for completeness — I believe the mobile native app uses a "bottom sheet" for the block library: https://www.material.io/design/components/sheets-bottom.html — I believe the reason we're not doing that here is that in web-mobile we can't control whether the soft keyboard is showing, and therefore anything viewport edge fixed is DOA. But mentioning in case someone can magic. (⊃。•́‿•̀。)⊃━☆゚.*・。゚

@youknowriad
Copy link
Contributor Author

Ok I need a final direction here. Should I replace with "Add a block"?

@melchoyce
Copy link
Contributor

Yeah, let's do "Add a block" 👍

@mapk
Copy link
Contributor

mapk commented Mar 20, 2019

"Add a block" it is then! Let's do it! 🚢

@youknowriad youknowriad merged commit c7345ee into master Mar 21, 2019
@youknowriad youknowriad deleted the remove/inserter-title branch March 21, 2019 07:39
@youknowriad youknowriad added this to the 5.4 (Gutenberg) milestone Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants