Feat: support for dynamic placeholders
Description
Added placeholderComponentsConfiguration parameter to allow us to build placeholders dynamically depending on what attribute we want and cursorParagrahPlaceholderConfiguration that allows to the devs add a placeholder at the right (if we are using a LTR language) of the cursor (only when the paragraph and it does not contains any block attribute or selection)
For example, if we only want to display placeholders for Headers and Lists, then we would set it up like this:
configurations: QuillEditorConfigurations(
// We can use this instance to avoid use this functionality:
// CursorParagrahPlaceholderConfiguration.noPlaceholder()
// Or, if we want to add our custom settings, we can configure manually
// CursorParagrahPlaceholderConfiguration(paragraphPlaceholderText: "our_text'', style: TextStyle(), show: true);
// At this example case, we use this instance (just for showcases)
cursorParagrahPlaceholderConfiguration: CursorParagrahPlaceholderConfiguration.withPlaceholder(),
placeholderComponentsConfiguration:
PlaceholderComponentsConfiguration(
builders: {
Attribute.header.key: (Attribute attr, style) {
final values = [30, 27, 22];
final level = attr.value as int?;
if (level == null) return null;
final fontSize = values[ (level - 1 < 0 || level - 1 > 3 ? 0 : level - 1)];
return PlaceholderArguments(
placeholderText: 'Header $level',
style: TextStyle(fontSize: fontSize.toDouble())
.merge(style.copyWith(color: Colors.grey)),
);
},
Attribute.list.key: (Attribute attr, style) {
return PlaceholderArguments(
placeholderText: 'List item',
style: style.copyWith(color: Colors.grey),
);
},
},
),
),
Preview
Web
https://github.com/user-attachments/assets/23e79d41-b194-41e4-aba4-839be1ec7724
Desktop
https://github.com/user-attachments/assets/1a36f67d-6ab7-4669-bf1c-4269476fcc7c
Android
https://github.com/user-attachments/assets/ccdc8eb8-61b9-4b48-9968-3634de73919e
Related Issues
N/A
- [x] โจ 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.
TODO
- [x] We must also make a cursor placeholder that appears at the same offset of the caret on any empty line (if the line has not block or embed attributes)
- [x] We must make possible add custom block attributes keys (we need to add some validations because we don't want to add any non block attribute)
- [x] Make this feature completely optional.
- [x] We need to add support for web
- [ ] Fix issue with the positioning of the placeholders in mobile devices
- [ ] Provide a way to calculate correctly the offset of the placeholder (only for cursor)
- [ ] Test it on all platforms (this is an issue by itself because i don't have IOS or MacOs devices)
- [ ] Test if the feature could cause any issue if we have a line with several block attributes applied
- [ ] Test the system and attribute directionality (RTL, and LTR)
- [ ] Test cursor placeholder
- [ ] Add documentation about this feature.
Seems to be an interesting feature.
I have noticed we're adding new features and then introduce breaking changes after short time to fix a related issue so we probably want to mark the new features as experimental for a while then make them stable similar to how it's done on other project.
For example, the SpellCheckerService needs to be completely rewritten. It should abstract the source of checking the spell of words and not a service for managing a client-side spell checker. The user may want to use server-side solution or a services that provide web API so they can send GET request and get the response back, or they might want to use method channel to access the system spell checker service, the other functions such as dispose and the others shouldn't be part of the interface instead should be only managed by the client side spell checker service provided by the library itself, or maybe we can provide helper class for that, we should only have one method that is a future similarly to Flutter, and that's a breaking change, there are ways make it backward compatible but it's not as clean as removing them and can be confusing.
We can use the experimental annotation from themeta package. It doesn't work similarly to how it works on other languages so the user can compile without opting in.
This PR doesn't seem to have similar issues, but just in case so, we don't have to deprecate them and support them for a while like the enableMarkdownStyleConversion.
This is an optional suggestion.
Thank @AtlasAutocode for the feedback, I do agree that we should test our changes in the dev branch and push them as pre-release since the testing is very manual. Marking them as experimental is the least we can do.
We should be more cautious when introducing new changes since medium-big projects depend on this project. Users expect the changes to be stable in the master or the stable release.
This is a mistake I made in 2023 when working on this repository since I didn't even have the time to write a good commit message or explain my changes (See #1566 as a perfect example). Now I'm introducing fewer features and working less often than before but all of my changes are much more stable now. At the end of 2023, I only had 1 hour in a month.
I'm able to reproduce bugs that are not in the issue that we can't keep track of and have been since 2023 (example).
For now, I suggest making most of our new changes in completely separate files to make it easier to keep track of things, we don't want to make the QuillRawEditorState and QuillController and related files 40% bigger.
Use part and part of, extensions, private functions, classes. We need more code reviews like before, see 177ddf6428295d725e87 as a recent example.
Once I'm done with #2230. I will separate the magnifier into its own file (#2026, #2026).
I do appreciate your time and efforts.
We can use the experimental annotation from themeta package. It doesn't work similarly to how it works on other languages so the user can compile without opting in.
Thanks for the suggestion. I'll take it into consideration.
This PR doesn't seem to have similar issues, but just in case so, we don't have to deprecate them and support them for a while like the enableMarkdownStyleConversion.
You are correct, there are no issues related to placeholders. However, I thought we could add this functionality to improve how we view elements that have attributes in blocks, such as lists, titles, and even blockquotes that when empty, do not display any text, which is sometimes confusing (at least in my case it has been).
I take BlockNote editor as an example of how this feature should be maded:
For example, the
SpellCheckerServiceneeds to be completely rewritten. It should abstract the source of checking the spell of words and not a service for managing a client-side spell checker. The user may want to use server-side solution or a services that provide web API so they can send GET request and get the response back, or they might want to use method channel to access the system spell checker service, the other functions such as dispose and the others shouldn't be part of the interface instead should be only managed by the client side spell checker service provided by the library itself, or maybe we can provide helper class for that, we should only have one method that is a future similarly to Flutter, and that's a breaking change, there are ways make it backward compatible but it's not as clean as removing them and can be confusing.
I will take this into account to improve the problems we are having with the current spell checker. Anyway, thanks for the suggestion.
For example, the SpellCheckerService needs to be completely rewritten.
Spell checking might be very important for some users, but the penalty is too high. The implementation is completely useless for me as I need a highly technical vocabulary that is never going to be implemented.
You have a point. But, some devs could need this feature. I'll add it to my TODO list. Thanks for your feedback.
Embedding Table
Still not implemented? Please fix or remove. Or at least mark as experimental so users know to avoid. This is a perfect example of working in a private branch until the feature is complete and ready for general use.
I think the same. Table should not be available for users. I'll make a PR to disable it by now because is quite experimental and it does not work as we expect. I'll working on it to improve our implementation.
placeholder appear on any empty line
NO!!!
It seems I expressed myself poorly. I blame my PC for not being able to properly record a sample video for this feature (no idea).
At this point, what I mean is that if we create our own placeholders using placeholderComponentsConfiguration whenever these lines are empty, their intended placeholder will appear. However, this is ONLY limited to the exclusive attributes i.e. header, list, code-block, and blockquote. Any other paragraph-like or inline attributes (alignment, line-height, direction and similar attributes are also ignored to avoid putting unwanted placeholders)
Now, we have the case of the placeholder that appears at the cursor level. This is limited to only appearing IF the line is empty and it does not have any block or embedded attributes. That is, it is a full-fledged paragraph.
Needless to say, by default, none of these features are enabled. It is up to the user to configure them. If they are not desired, one can simply pass null, or not call them in the constructor.
[!NOTE] If you have time, please, check BlockNote. This feature works in a very similar way to how this editor works with its placeholders.
I'm having a bad day. I apologize if my comments are too harsh. I'm getting frustrated.
Don't worry, man. There are always those kinds of days. Thanks for your feedback, I'll take it into account for any future PR I'm going to do.
I agree with @EchoEllet that new features should be complete before submitting. Bugs and errors in edge cases are inevitable but code needs to be tested before submitting. Code used by our userbase must be stable and complete.
I completely agree. That's why many of the PRs I've done take their time to be tested and to ensure that there are no problems. Anyway, thanks for mentioning it.
I will take this into account to improve the problems we are having with the current spell checker. Anyway, thanks for the suggestion.
I do suggest that we discuss it before attempting to introduce any changes, I do plan to at least try to use the DefaultSpellCheckerService from Flutter. if it doesn't work will implement it using quill_native_bridge.
You have a point. But, some devs could need this feature. I'll add it to my TODO list. Thanks for your feedback.
Another mistake I made in 2023 is introducing any feature that is needed by all developers. If this is not something commonly known and used then it shouldn't be a feature since it can cause issues for the other 95% developers. Instead, we should make it possible to implement to the 5% developers in a clean way that doesn't cause breaking changes in the future. Having an 80 MB file on a client app (especially on the web) is not ideal which is why is one reason why I moved it into the example. Even if this is fixed after very short time, it can give a negative impression to users, which is the main reason I suggest discussing this before proceeding further, and another reason why I invited you to the development channel on Slack recently. Different developers have experience in different areas.
I completely agree. That's why many of the PRs I've done take their time to be tested and to ensure that there are no problems. Anyway, thanks for mentioning it.
Sure but not all issues can be encountered by testing the feature in working time. Today I was able to detect an issue on Android that only occurs when copying an image from another app and when pasting it, then closing/restarting the app and attempting to paste it once again.
This will cause a security exception to be thrown each time opening the app, and the only workaround is to ask the user to clear their clipboard or copy something different and we can't since the app won't open.
the app will keep crashing infinitely until the user deletes the app or the system asks them to. This was an issue in quill_native_bridge and super_clipbaord (almost all clipboard plugins that support Android + some native Android apps).
I provided a workaround (return null instead of crashing the whole app - disable the feature) I wouldn't be able to detect this (in addition to many other issues) if I didn't 4 days for #2230. I could have marked it as ready for merge/review on the first day and then submitted PRs fixing bugs and incomplete features. Every user/developer will be a test site.
See super_native_extensions #435 for more details about this issue.
[!NOTE] Used www.github.com instead of github.com in the link above to avoid linking/referencing unrelated issues on their repo
The issue with the indent was moved to #2253
I apologize for the inactivity I have had. Time has been quite tight and it has been difficult for me to continue contributing.
In one or two days I will start again, resume activity and continue with the PRs that I have active
I apologize for the inactivity I have had. Time has been quite tight and it has been difficult for me to continue contributing.
No need to apologize. Even if you don't continue, you've contributed well, and I'm grateful.
In one or two days I will start again, resume activity and continue with the PRs that I have active
There's no urgency, so start whenever you're ready and want to.
Same as previous comment. How do you prefer to handle those conflicts?
@EchoEllet as i said before, i have no problem with you making a new PR if necessary. Thank you very much for taking the time and effort.
I appreciate your contributions, but please consider creating issues before working on them because I'm not with the idea of adding this feature or any new similar features. The editor is just not stable.
It was much better before, and so far, we haven't addressed many regressions affecting production apps. Users have already started to migrate away from the project and use alternatives.
See #2445 issue as one of mant examples, they are also many major bugs that aren't being reported.
Adding new features that cause issues will require others to take responsibility for maintaining them. We don't have enough tests, and we have already discussed this many times.
I prefer stability, especially with the current state, even if the progress is slower. If we can't fix the current issues, then it's better not to make the project any less stable.
I will create one issue for such discussions because I don't want to repeat it everywhere.
@singerdmx I will leave the decision to you as I don't have a very strong opinion about the close.
Adding features that we're not certain about, then fixing them later once users start reporting them comes at a high price.
@EchoEllet I didn't plan on adding this feature at the moment. I just wanted to verify that everything was in order and finish the changes. I have the same opinion on this point, I would prefer this feature to be merged after solving most of the bugs we have at the moment.
@singerdmx by now, i would prefer that this PR remain unmerged for now, and wait for a much more stable version without so many issues of the project.
I would prefer this feature to be merged after solving most of the bugs we have at the moment.
There is no ETA for a more stable version. Last year, we discussed this, and it's seems everyone excited and has high expectations, but all that happens is that the magnifier was introduced with many major bugs and fixed yesterday after many months, which simply makes the experience similar or even less stable than before.
I would prefer this feature to be merged after solving most of the bugs we have at the moment.
Some users are using older versions and sticking to them due to such issues, and this is not the kind of developer and user experience I'm looking forward to.
And because of that, we're not certain if this PR will ever get merged, which is why I suggest creating issues for future PRs. I'm mainly concerned we end up closing many PRs that would discourage contributors with the time they have already spent.
I will close the PR as not only are we not in a position to add new features at the moment, but it will also likely be a long time before it is even merged into the stable branch.