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

Feat: custom leading builder #2146

Merged
merged 14 commits into from
Aug 30, 2024
Merged

Conversation

CatHood0
Copy link
Collaborator

@CatHood0 CatHood0 commented Aug 25, 2024

Description

A new property has been added that will allow us to configure the leading of the elements in a list without any limitations.

Example of a custom bullet list:

This code:

 customLeadingBlockBuilder: (node, config) {
     if (config.attribute == Attribute.ul) {
            final indent = (config.attrs[Attribute.indent.key]?.value as int?) ?? 0;
             return Container(
                  alignment: AlignmentDirectional.topEnd,
                  padding: EdgeInsetsDirectional.only(end: config.padding ?? 0),
                  child: Text(
                       indent == 0
                           ? '😃'
                           : indent == 1
                                ? '👽'
                                : indent == 2
                                     ? '♥︎'
                                     : indent == 3
                                         ? '🫸'
                                         : indent == 4
                                             ? '🌏'
                                             : '🤕',
                            style: TextStyle(fontSize: 13),
               ),
          );
     }
     return null;
},

Will have this effect on the view:

Captura de pantalla_2024-08-26_12-36-49

Related Issues

Not related issues

These issues are not related but was fixed while creating the feature.

  • New feature: Adds new functionality without breaking existing features.
  • 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Code refactor: Code restructuring that does not affect behavior.
  • Breaking change: Alters existing functionality and requires updates.
  • 🧪 Tests: Adds new tests or modifies existing tests.
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • Build configuration change: Changes to build or deploy processes.

Suggestions

We should stop creating such hardcoded code as it is really getting complicated trying to create default blocks to give users the opportunity to create their own independent components.

@CatHood0 CatHood0 requested review from EchoEllet and singerdmx and removed request for singerdmx and EchoEllet August 25, 2024 02:19
@CatHood0 CatHood0 marked this pull request as draft August 25, 2024 02:21
@singerdmx
Copy link
Owner

Is the node inline or block level?

@singerdmx
Copy link
Owner

OK, I got the idea.
Looking forward to see how it turns out.

@CatHood0
Copy link
Collaborator Author

CatHood0 commented Aug 25, 2024

Is the node inline or block level?

I trying to create builders for both types (i'm first trying for inline type, and if it works, then i will make the same for block type). I pass the TextLine as an argument of TextLineNodeBuilder to let to users wrap it, but, it cannot be wrapped since EditableTextLine expect another type of widget.

@CatHood0
Copy link
Collaborator Author

CatHood0 commented Aug 26, 2024

@singerdmx Do you got any idea on how to use TextLine wrapped in another type of widget? I saw that RenderContentProxyBox has methods that are used specifically for RenderEditableTextLine. If not, I'll have to just limit the Feature to being a Custom builder for Leading Lists and Code Blocks.

@CatHood0
Copy link
Collaborator Author

@EchoEllet how can i get the analizer ignore the deprecated controller param in editor_onfigurations file?

@EchoEllet
Copy link
Collaborator

EchoEllet commented Aug 26, 2024

@EchoEllet how can i get the analizer ignore the deprecated controller param in editor_onfigurations file?

Add a comment before the statement:

// ignore: deprecated_member_use_from_same_package

If this isn’t quite what you meant, I might need a few more details. I’m currently tied up with something else and haven’t had a chance to review this PR yet.

@CatHood0
Copy link
Collaborator Author

@EchoEllet thank you. There's no problem, when you have time just take a look.

@CatHood0
Copy link
Collaborator Author

I'll give up for now because the code is very dependent on certain types that don't allow me to quickly figure out how to correctly implement NodeBuilders. I'll try to implement them later, since I don't have much time for now and it's better to leave this PR as a feature so that the list leaders can be modified without any complications.

@CatHood0 CatHood0 marked this pull request as ready for review August 28, 2024 03:20
@CatHood0 CatHood0 changed the title Feat: custom node builders Feat: custom leading builder Aug 28, 2024
@CatHood0 CatHood0 added the enhancement New feature or request label Aug 28, 2024
@singerdmx
Copy link
Owner

Yeah we can leave it and maybe someone else may be able to help

@singerdmx
Copy link
Owner

This is a big change and it is not so easy

@CatHood0 CatHood0 marked this pull request as draft August 28, 2024 11:21
@vishna
Copy link
Collaborator

vishna commented Aug 28, 2024

@CatHood0 tried this PR to achieve the bullet list I mentioned in other ticket, works much nicer already 🎉 good work 🙇

Screenshot 2024-08-28 at 15 39 07

My remarks:

  • the base width of the leading container is not customizable but calculated based on EditableTextBlock._numberPointWidth or just font size
  • LeadingConfigurations.width - relying on this to make the positioning look acceptable - it's a nullable, can it ever be null though? 🤔
      width: isOrdered || isCodeBlock
          ? _numberPointWidth(fontSize, count)
          : isUnordered
              ? fontSize * 2
              : null, // can this happen ?

It would be nice to have a custom builder for the width (and then for the deeper levels' indentation width - I didn't figure out yet which bit is adding this )

@CatHood0 CatHood0 marked this pull request as ready for review August 28, 2024 19:40
@CatHood0
Copy link
Collaborator Author

CatHood0 commented Aug 28, 2024

the base width of the leading container is not customizable but calculated based on EditableTextBlock._numberPointWidth or just font size

  1. It is calculated by this way because it allows work with the default implementation.

LeadingConfigurations.width - relying on this to make the positioning look acceptable - it's a nullable, can it ever be null though? 🤔

  1. Yes, it can be null if the list is of type Checkbox.

It would be nice to have a custom builder for the width (and then for the deeper levels' indentation width - I didn't figure out yet which bit is adding this )

Actually, the width of the TextBlock is not that easy to implement. By now width you is applied to the leading. You can always use the width in your own widgets or not use it, since they are customizable.

@vishna
Copy link
Collaborator

vishna commented Aug 28, 2024

Can't you just use the width that you want?

And the width of the Leading just affects to the leading and not the entire block.

That's the point, the bounding box for whatever container I build with customLeadingBlockBuilder seems to be predetermined and will be LeadingConfigurations.width (or more for upper level). I'll try to hack something tomorrow as I want to be able to customize the space available for the customLeadingBlockBuilder

@vishna
Copy link
Collaborator

vishna commented Aug 29, 2024

👋 @CatHood0

made a PR to your PR here: CatHood0#1 I hope this explains my points better 😅 Also it seems to work just fine, other test pass and I'm able to have this aspect of the library now fully customized 🎉

@singerdmx
Copy link
Owner

@CatHood0 is this ready for merging?

@vishna
Copy link
Collaborator

vishna commented Aug 29, 2024

@singerdmx @CatHood0 I'd be grateful to have my changes also in 🙇

vishna and others added 2 commits August 29, 2024 13:37
#1)

* add ability to customize leading point width and leading point indent width

* fix one minor discrepancy
@CatHood0
Copy link
Collaborator Author

@singerdmx this can be merged now-

@singerdmx singerdmx merged commit 87284c8 into singerdmx:master Aug 30, 2024
2 checks passed
@singerdmx
Copy link
Owner

@CatHood0 it would be nice if there are some updates to README about this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants