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

[Bug] HeadingBlockComponentBuilder doesn't use placeholderTextStyle #663

Closed
armandsLa opened this issue Jan 10, 2024 · 4 comments · Fixed by #677
Closed

[Bug] HeadingBlockComponentBuilder doesn't use placeholderTextStyle #663

armandsLa opened this issue Jan 10, 2024 · 4 comments · Fixed by #677

Comments

@armandsLa
Copy link

Bug Description

The placeholderTextStyle under configuration of HeadingBlockComponentBuilder is not being used. The placeholder is visible, but the text style is the same as it's with normal text.

How to Reproduce

  1. Add placeholderText and placeholderTextStyle to the HeadingBlockComponentBuilder
  2. Open Heading menu and select Header 1

Expected Behavior

placeholderTextStyle to be used.

Operating System

Flutter/iOS

AppFlowy Editor Version(s)

2.2.0

Screenshots

No response

Additional Context

No response

@LucasXu0
Copy link
Collaborator

LucasXu0 commented Jan 11, 2024

I'm unable to reproduce it. Can you share your code sample?

Screenshot 2024-01-11 at 12 50 48

@armandsLa
Copy link
Author

Sure, here is a screenshot:

Screenshot 2024-01-11 at 14 46 16

And the method itself:

Map<String, BlockComponentBuilder> _makeBlockComponentBuilders(ActivityNotesState state) {
    return <String, BlockComponentBuilder>{
      PageBlockKeys.type: PageBlockComponentBuilder(),
      ParagraphBlockKeys.type: ParagraphBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          placeholderTextStyle: (_) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
            color: ThemeProvider.current.lightText,
          ),
          placeholderText: (node) => tr("activityNotes.empty"),
        ),
        showPlaceholder: (state, node) {
          final selectionStart = state.selection?.start.path.lastOrNull ?? 0;
          return selectionStart == node.path.lastOrNull;
        },
      ),
      BulletedListBlockKeys.type: BulletedListBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          placeholderTextStyle: (_) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
            color: ThemeProvider.current.lightText,
          ),
          placeholderText: (_) => tr("activityNotes.empty"),
        ),
      ),
      NumberedListBlockKeys.type: NumberedListBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          placeholderTextStyle: (_) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
            color: ThemeProvider.current.lightText,
          ),
          placeholderText: (_) => tr("activityNotes.empty"),
        ),
        iconBuilder: (context, node) => EditorNumberWidget(
          node: node,
          textStyle: ThemeProvider.current.textTheme.bodyLarge!,
          direction: Directionality.of(context),
        ),
      ),
      QuoteBlockKeys.type: QuoteBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          placeholderTextStyle: (_) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
            color: ThemeProvider.current.lightText,
          ),
          placeholderText: (_) => tr("activityNotes.empty"),
        ),
        iconBuilder: (context, _) => EditorQuoteWidget(color: state.themeColor),
      ),
      HeadingBlockKeys.type: HeadingBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          padding: (_) => EdgeInsets.zero,
          placeholderTextStyle: (_) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
            color: Colors.red,
          ),
          placeholderText: (node) => "Placeholder",
        ),
        textStyleBuilder: (level) => ThemeProvider.current.textTheme.bodyLarge!.copyWith(
          fontSize: EditorConstants.headingSizes.elementAtOrNull(level - 1) ?? 16.0,
          fontWeight: level == 1 ? FontWeight.w600 : FontWeight.w500,
        ),
      ),
      DividerBlockKeys.type: DividerBlockComponentBuilder(
        configuration: BlockComponentConfiguration(
          padding: (node) => EdgeInsets.symmetric(vertical: 8.0),
        ),
        lineColor: ThemeProvider.current.text
      ),
    };
  }

@armandsLa armandsLa changed the title HeadingBlockComponentBuilder doesn't use placeholderTextStyle [Bug] HeadingBlockComponentBuilder doesn't use placeholderTextStyle Jan 11, 2024
@LucasXu0
Copy link
Collaborator

I copied your code into the example project, and it works well there too. Did I miss anything?

Screenshot 2024-01-11 at 22 35 38

@armandsLa
Copy link
Author

The problem exists when textStyleBuilder has a style that also has a textColor. Color specified in configuration is overridden by that value. You can see the problem in _HeadingBlockComponentWidgetState class, line 153:

              placeholderTextSpanDecorator: (textSpan) => textSpan
                  .updateTextStyle(
                    placeholderTextStyle,
                  )
                  .updateTextStyle(
                    widget.textStyleBuilder?.call(level) ??
                        defaultTextStyle(level),
                  ),

I think it's a bug. I see no reason why styles from textStyleBuilder should be applied to placeholders. If it's done, then it should be in reverse - first styles from textStyleBuilder, then from placeholderTextStyle. I hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants