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 for Placeholder that is missing on heading block #739

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions __mocks__/styleMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ module.exports = {
blockHolderFocused: {
borderColor: 'gray',
},
'wp-block-heading': {
minHeight: 60,
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import android.graphics.Typeface;
import android.support.annotation.Nullable;
import android.text.Editable;
import android.text.TextUtils;
import android.text.TextWatcher;
import android.util.Log;
import android.util.TypedValue;
Expand Down Expand Up @@ -63,6 +64,8 @@ public class ReactAztecManager extends SimpleViewManager<ReactAztecText> {

private static final String TAG = "ReactAztecText";

private static final String BLOCK_TYPE_TAG_KEY = "tag";

public ReactAztecManager() {
initializeFocusAndBlurCommandCodes();
}
Expand Down Expand Up @@ -302,6 +305,13 @@ public void setColor(ReactAztecText view, @Nullable Integer color) {
view.setTextColor(newColor);
}

@ReactProp(name = "blockType")
public void setBlockType(ReactAztecText view, ReadableMap inputMap) {
if (inputMap.hasKey(BLOCK_TYPE_TAG_KEY)) {
view.setTagName(inputMap.getString(BLOCK_TYPE_TAG_KEY));
}
}

@ReactProp(name = "placeholder")
public void setPlaceholder(ReactAztecText view, @Nullable String placeholder) {
view.setHint(placeholder);
Expand Down Expand Up @@ -489,13 +499,14 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
return;
}

int currentEventCount = mEditText.incrementAndGetEventCounter();
// The event that contains the event counter and updates it must be sent first.
// TODO: t7936714 merge these events
mEventDispatcher.dispatchEvent(
new ReactTextChangedEvent(
mEditText.getId(),
mEditText.toHtml(false),
mEditText.incrementAndGetEventCounter()));
currentEventCount));

mEventDispatcher.dispatchEvent(
new ReactTextInputEvent(
Expand All @@ -504,6 +515,11 @@ public void onTextChanged(CharSequence s, int start, int before, int count) {
oldText,
start,
start + before));

// Add the outer tags when the field was started empty, and only the first time the user types in it.
if (mPreviousText.length() == 0 && currentEventCount == 1 && !TextUtils.isEmpty(mEditText.getTagName())) {
Copy link
Contributor

@marecar3 marecar3 Mar 13, 2019

Choose a reason for hiding this comment

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

If you agree we can probably put mPreviousText.isEmpty() instad of mPreviousText.length() == 0
also it would be good to put currentEventCount == 1 in some constant with self-explaining name.

Copy link
Contributor Author

@daniloercoli daniloercoli Mar 14, 2019

Choose a reason for hiding this comment

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

Unfortunately there are cases where Aztec does only contain "non-breaking-space" char and isEmpty does return true, but the length is not 0. This is happening when you write and then remove all the content from the block.

Will address the other comments instead.

mEditText.fromHtml('<' + mEditText.getTagName() + '>' + newText + "</" + mEditText.getTagName() + '>', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there maybe some Util that we can use to generate the html from tag name and text in order to have one place for adding the <,> when there is a need for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that i'm aware of, but in this case it's pretty simple the input string, that doesn't worth introduce a new function for it. We can use StringBuilder, but it's a very simple string.

}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.events.EventDispatcher;
import com.facebook.react.views.textinput.ContentSizeWatcher;
import com.facebook.react.views.textinput.ReactTextChangedEvent;
import com.facebook.react.views.textinput.ReactTextInputLocalData;
import com.facebook.react.views.textinput.ScrollWatcher;

Expand All @@ -35,7 +34,6 @@
import java.util.LinkedList;
import java.util.Set;
import java.util.HashSet;
import java.util.Map;
import java.util.HashMap;

import static android.content.ClipData.*;
Expand Down Expand Up @@ -66,6 +64,11 @@ public class ReactAztecText extends AztecText {
boolean shouldHandleOnSelectionChange = false;
boolean shouldHandleActiveFormatsChange = false;

// This optional variable holds the outer HTML tag that will be added to the text when the user start typing in it
// This is required to keep placeholder text working, and start typing with styled text.
// Ref: https://github.com/wordpress-mobile/gutenberg-mobile/issues/707
private String mTagName = "";

private static final HashMap<ITextFormat, String> typingFormatsMap = new HashMap<ITextFormat, String>() {
{
put(AztecTextFormat.FORMAT_BOLD, "bold");
Expand Down Expand Up @@ -241,6 +244,14 @@ public void run() {
setIntrinsicContentSize();
}

public void setTagName(@Nullable String tagName) {
this.mTagName = tagName;
}

public String getTagName() {
return this.mTagName;
}

private void updateToolbarButtons(int selStart, int selEnd) {
ArrayList<ITextFormat> appliedStyles = getAppliedStyles(selStart, selEnd);
updateToolbarButtons(appliedStyles);
Expand Down
2 changes: 1 addition & 1 deletion src/_variables.android.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ $title-block-padding-bottom: 0;

$min-height-title: 53;
$min-height-paragraph: 50;
$min-height-heading: 50;
$min-height-heading: 60;