-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Editable manages the viewport display #338
Conversation
|
484c9fc
to
d94330d
Compare
d94330d
to
fe91995
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #338 +/- ##
==========================================
+ Coverage 87.90% 87.97% +0.07%
==========================================
Files 64 61 -3
Lines 10382 11105 +723
==========================================
+ Hits 9126 9770 +644
- Misses 1256 1335 +79 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works mostly fine but I'm facing problem when using the editor or field in a scrolling view without limiting its height. Here is an example which works fine with master:
import 'package:fleather/fleather.dart';
import 'package:flutter/material.dart';
void main() {
runApp(const FleatherApp());
}
class FleatherApp extends StatelessWidget {
const FleatherApp({Key? key}) : super(key: key);
@override
Widget build(BuildContext context) => MaterialApp(
debugShowCheckedModeBanner: false,
theme: ThemeData.light(),
darkTheme: ThemeData.dark(),
title: 'Fleather - rich-text editor for Flutter',
home: HomePage(),
);
}
class HomePage extends StatefulWidget {
const HomePage({Key? key}) : super(key: key);
@override
_HomePageState createState() => _HomePageState();
}
class _HomePageState extends State<HomePage> {
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(elevation: 0, title: Text('Fleather Demo')),
body: SingleChildScrollView(
child: Padding(
padding: const EdgeInsets.all(8.0),
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
_Bubble('Test Test Test'),
Align(
alignment: Alignment.topRight,
child: _Bubble('Test Test Test Test Test Test Test Test'),
),
_Bubble('Test Test Test'),
Align(
alignment: Alignment.topRight,
child: _Bubble('Test Test Test Test Test Test Test Test'),
),
_Bubble('Test Test Test'),
Align(
alignment: Alignment.topRight,
child: _Bubble('Test Test Test Test Test Test Test Test'),
),
],
),
),
),
);
}
}
class _Bubble extends StatelessWidget {
final String text;
const _Bubble(this.text);
@override
Widget build(BuildContext context) {
return Container(
decoration: BoxDecoration(
color: Colors.grey.shade300,
borderRadius: BorderRadius.circular(8),
),
constraints: BoxConstraints(maxWidth: 200),
padding: EdgeInsets.all(8),
child: IntrinsicWidth(
child: FleatherEditor(
controller: FleatherController(
document: ParchmentDocument.fromJson([
{'insert': '$text\n'}
]))),
),
);
}
}
Thanks @Amir-P for the example. All in all I propose to remove the assert and from now, to implement the desired behavior, you should pass WDYT? |
@amantoux TBH I didn't check the code at that time. Your suggestion sounds reasonable to me. Anyway, I found another problem that might be good to look into. import 'dart:convert';
import 'dart:io';
import 'package:fleather/fleather.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
void main() {
runApp(const FleatherApp());
}
class FleatherApp extends StatelessWidget {
const FleatherApp({Key? key}) : super(key: key);
@override
Widget build(BuildContext context) => MaterialApp(
debugShowCheckedModeBanner: false,
theme: ThemeData.light(),
darkTheme: ThemeData.dark(),
title: 'Fleather - rich-text editor for Flutter',
home: HomePage(),
);
}
class HomePage extends StatefulWidget {
const HomePage({Key? key}) : super(key: key);
@override
_HomePageState createState() => _HomePageState();
}
class _HomePageState extends State<HomePage> {
final FocusNode _focusNode = FocusNode();
FleatherController? _controller;
@override
void initState() {
super.initState();
if (kIsWeb) BrowserContextMenu.disableContextMenu();
_initController();
}
@override
void dispose() {
super.dispose();
if (kIsWeb) BrowserContextMenu.enableContextMenu();
}
Future<void> _initController() async {
try {
final result = await rootBundle.loadString('assets/welcome.json');
final doc = ParchmentDocument.fromJson(jsonDecode(result));
_controller = FleatherController(document: doc);
} catch (err, st) {
print('Cannot read welcome.json: $err\n$st');
_controller = FleatherController();
}
setState(() {});
}
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(elevation: 0, title: Text('Fleather Demo')),
body: _controller == null
? Center(child: const CircularProgressIndicator())
: SingleChildScrollView(
child: Column(
children: [
FleatherToolbar.basic(controller: _controller!),
Divider(height: 1, thickness: 1, color: Colors.grey.shade200),
// FleatherEditor(
// controller: _controller!,
// focusNode: _focusNode,
// padding: EdgeInsets.only(
// left: 16,
// right: 16,
// bottom: MediaQuery.of(context).padding.bottom,
// ),
// onLaunchUrl: _launchUrl,
// maxContentWidth: 800,
// embedBuilder: _embedBuilder,
// spellCheckConfiguration: SpellCheckConfiguration(
// spellCheckService: DefaultSpellCheckService(),
// misspelledSelectionColor: Colors.red,
// misspelledTextStyle:
// DefaultTextStyle.of(context).style),
// ),
TextField(
controller: TextEditingController(
text: _controller!.document.toPlainText()),
maxLines: null,
),
],
),
),
);
}
Widget _embedBuilder(BuildContext context, EmbedNode node) {
if (node.value.type == 'icon') {
final data = node.value.data;
// Icons.rocket_launch_outlined
return Icon(
IconData(int.parse(data['codePoint']), fontFamily: data['fontFamily']),
color: Color(int.parse(data['color'])),
size: 18,
);
}
if (node.value.type == 'image') {
final sourceType = node.value.data['source_type'];
ImageProvider? image;
if (sourceType == 'assets') {
image = AssetImage(node.value.data['source']);
} else if (sourceType == 'file') {
image = FileImage(File(node.value.data['source']));
} else if (sourceType == 'url') {
image = NetworkImage(node.value.data['source']);
}
if (image != null) {
return Padding(
// Caret takes 2 pixels, hence not symmetric padding values.
padding: const EdgeInsets.only(left: 4, right: 2, top: 2, bottom: 2),
child: Container(
width: 300,
height: 300,
decoration: BoxDecoration(
image: DecorationImage(image: image, fit: BoxFit.cover),
),
),
);
}
}
return defaultFleatherEmbedBuilder(context, node);
}
} |
@Amir-P Here is the behavior I propose:
If
WDYT? |
Sorry Alan I was busy. Will try to get back to you today. @amantoux |
The behavior you've described sounds good to me, Alan. Do you need to make changes? @amantoux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks.
Hi, I've been having an issue and it seems to relate to the behaviour listed above. I'm trying to use FleatherField like I would a normal TextFormField, but the cursor always ends up underneath the keyboard. Other than this one issue I'm loving Fleather, but this is unfortunately blocking me, as we have multiple editable fields in a scrollable list, so they need to respect keyboard correctly. Reading the above I hoped the issue was resolved but doesn't seem to be from my testing. @Amir-P was the issue you found with scrolling fixed as part of this PR? |
Hey @kane-knowby, Thanks for bringing this to our attention. The issue you are referring to wasn't fixed with this PR since I realized that we had the issue even before this PR. But it's something that we need to address. Can you please create an issue for it? |
This PR is an attempt to mimic Flutter's
Editable
that behaves as a viewport (invisible elements are clipped out).The benefit here is that it eases future optimizations that would prevent from rendering element that are not visible.
It also allows us to avoid the need to define a custom
SingleChildScrollView