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

Try removing abs position on placeholder text in button #1247

Closed
wants to merge 3 commits into from

Conversation

jasmussen
Copy link
Contributor

This tries to address #1168, specifically the concerns around letting the placeholder text scale the content.

Props @georgeolaru, #1168 (comment)

The downside is that the cursor is blinking at the end:

placeholder

But as soon as you start typing, it works as intended. Can we address this? Can we set the caret at the beginning using some trick? Stacked textareas, one left floated? @iseulde any ideas?

This tries to address #1168, specifically the concerns around letting the placeholder text scale the content.
@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label Jun 19, 2017
@jasmussen jasmussen self-assigned this Jun 19, 2017
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Can we set the caret at the beginning using some trick?

My first thought is changing :before to :after for the placeholder, which would work except for the fact that there's a "bogus" linebreak added by TinyMCE in the case of empty content. If we change the placeholder to not be absolute-ly positioned, we wouldn't need the line break. Not sure yet how to go about preventing it from being added.

@@ -27,6 +27,10 @@ $blocks-button__height: 46px;
left: 50%;
transform: translateX( -50% );
}

.blocks-editable__tinymce[data-is-empty="true"]:before {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be so specific to button, or should we set this as the default for all empty Editable placeholders?

https://github.com/WordPress/gutenberg/blob/cab8713/blocks/editable/style.scss#L49

@aduth
Copy link
Member

aduth commented Jun 19, 2017

Tried changing to :after in fbc091c with adjusted positioning to move the pseudo-element "up" to the first line. This is the best compromise in my experimenting. It wouldn't work to remove or hide the <br /> because while this will show the placeholder text in the correct location, it won't show the text caret at all.

@aduth aduth added [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable and removed [Status] In Progress Tracking issues with work in progress labels Jun 19, 2017
@aduth
Copy link
Member

aduth commented Jun 19, 2017

The change in fbc091c seems to have resurfaced the issue described in #868 , resulting in double dashes on quote placeholder:

Quote placeholder double dash

We'll likely want to choose between one or the other between the placeholder dash added in 1975200 and the dash applied by styling.

@jasmussen
Copy link
Contributor Author

Thanks so much for the help!

We'll likely want to choose between one or the other between the placeholder dash added in 1975200 and the dash applied by styling.

Since adding that dash for this particular quote style is a, well, style choice, I lean towards us adding this in CSS.

Placeholder applied by styles

Closes #868
@aduth
Copy link
Member

aduth commented Jun 20, 2017

37b217f removes the redundant placeholder dash. Seeing now some issues with inserting a quote, assigning some citation text, then clearing it out. Appears the br[data-mce-bogus] isn't always consistently inserted.

@jasmussen
Copy link
Contributor Author

@iseulde if you have bandwidth do you have any knowledge on the behavior of data-mce-bogus? Thank you.

@ellatrix
Copy link
Member

I'll have a look. If the behaviour is inconsistent, that seems to be a TinyMCE bug.

@ellatrix
Copy link
Member

I think the best solution might be to have the placeholder as the content, and still make it seem as empty. This will also make sure we don't have any issues with something else styling :before or :after. I'll have a look at how doable this is.

@ellatrix
Copy link
Member

@spocke I don't see any placeholder settings or plugins for TinyMCE. Do you think there's any good way to build this in?

@BoardJames
Copy link

There doesn't appear to be an official placeholder-text plugin but there is one that turns up on a search ( https://github.com/mohan/tinymce-placeholder ). It appears that it just displays a label over the editor when the content is empty and hides it when it is shown.

@BoardJames
Copy link

One problem with this implementation is that once the block has been unfocused (by selecting another block) it is challenging to refocus the button label editor with the mouse. The plugin linked detects focus on the label and selects the editor instead which makes this work a bit better.

@ellatrix
Copy link
Member

Working on an alternative approach right now. Let's see.

@ellatrix
Copy link
Member

Fixed in #1477.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants