alexeyinkin commented on code in PR #23662:
URL: https://github.com/apache/beam/pull/23662#discussion_r997129095
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/unit_content.dart:
##########
@@ -30,6 +32,76 @@ class UnitContentWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
- return MarkdownBody(data: unitContent.description);
+ final textTheme = Theme.of(context).textTheme;
+ final markdownStyle = MarkdownStyleSheet(
+ h1: textTheme.headlineLarge,
+ h3: textTheme.headlineMedium,
+ p: textTheme.bodyMedium,
+ code: Theme.of(context)
+ .extension<BeamThemeExtension>()!
+ .codeRootStyle
+ .copyWith(
+ backgroundColor: BeamColors.transparent,
+ fontSize: BeamSizes.size12,
+ ),
+ codeblockDecoration: BoxDecoration(
+ // TODO(nausharipov): where to store these colors?
Review Comment:
`Beam*ThemeColors.codeBackground` are fine.
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/content.dart:
##########
@@ -52,12 +52,9 @@ class ContentWidget extends StatelessWidget {
mainAxisAlignment: MainAxisAlignment.spaceBetween,
children: [
Expanded(
- child: SingleChildScrollView(
- controller: ScrollController(),
Review Comment:
Controllers must never be created in build methods because build methods may
be called at any time, and so the scrolling will be lost.
In `TourNotifier`, make a map of `unitId -> ScrollController` and then pick
the controller in this widget with a method like
`getOrCreateScrollController(String unitId)`.
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/unit_content.dart:
##########
@@ -30,6 +32,76 @@ class UnitContentWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
- return MarkdownBody(data: unitContent.description);
+ final textTheme = Theme.of(context).textTheme;
+ final markdownStyle = MarkdownStyleSheet(
+ h1: textTheme.headlineLarge,
+ h3: textTheme.headlineMedium,
+ p: textTheme.bodyMedium,
+ code: Theme.of(context)
+ .extension<BeamThemeExtension>()!
+ .codeRootStyle
+ .copyWith(
+ backgroundColor: BeamColors.transparent,
+ fontSize: BeamSizes.size12,
+ ),
+ codeblockDecoration: BoxDecoration(
+ // TODO(nausharipov): where to store these colors?
+ color: Theme.of(context).brightness == Brightness.dark
+ ? BeamDarkThemeColors.codeBackground
+ : BeamLightThemeColors.codeBackground,
+ border: Border.all(color: Theme.of(context).primaryColor),
+ borderRadius: const BorderRadius.all(
+ Radius.circular(BeamSizes.size4),
+ ),
+ ),
+ );
+
+ return Markdown(
+ selectable: true,
+ data: unitContent.description,
+ builders: {
+ 'code': CodeBuilder(),
+ },
+ styleSheet: markdownStyle,
+ );
+ }
+}
+
+class CodeBuilder extends MarkdownElementBuilder {
+ @override
+ Widget? visitElementAfter(md.Element element, TextStyle? preferredStyle) {
+ final String textContent = element.textContent;
+ final bool isMultilineCode = textContent.contains('\n');
+ if (isMultilineCode) {
+ /// codeblockDecoration is applied for multiline code
+ return null;
+ }
+ return _InlineCode(text: textContent);
+ }
+}
+
+// TODO(nausharipov): is inline the right word?
+class _InlineCode extends StatelessWidget {
+ final String text;
+ const _InlineCode({required this.text});
+
+ @override
+ Widget build(BuildContext context) {
+ return Container(
+ padding: const EdgeInsets.symmetric(
+ horizontal: BeamSizes.size3,
+ vertical: BeamSizes.size1,
+ ),
+ decoration: BoxDecoration(
+ color: Theme.of(context).brightness == Brightness.dark
Review Comment:
Add `BeamThemeExtension.codeBackgroundColor` instead of checking brightness
here.
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/unit_content.dart:
##########
@@ -30,6 +32,76 @@ class UnitContentWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
- return MarkdownBody(data: unitContent.description);
+ final textTheme = Theme.of(context).textTheme;
+ final markdownStyle = MarkdownStyleSheet(
+ h1: textTheme.headlineLarge,
+ h3: textTheme.headlineMedium,
+ p: textTheme.bodyMedium,
+ code: Theme.of(context)
+ .extension<BeamThemeExtension>()!
+ .codeRootStyle
+ .copyWith(
+ backgroundColor: BeamColors.transparent,
+ fontSize: BeamSizes.size12,
+ ),
+ codeblockDecoration: BoxDecoration(
+ // TODO(nausharipov): where to store these colors?
+ color: Theme.of(context).brightness == Brightness.dark
+ ? BeamDarkThemeColors.codeBackground
+ : BeamLightThemeColors.codeBackground,
+ border: Border.all(color: Theme.of(context).primaryColor),
+ borderRadius: const BorderRadius.all(
+ Radius.circular(BeamSizes.size4),
+ ),
+ ),
+ );
+
+ return Markdown(
+ selectable: true,
+ data: unitContent.description,
+ builders: {
+ 'code': CodeBuilder(),
+ },
+ styleSheet: markdownStyle,
+ );
+ }
+}
+
+class CodeBuilder extends MarkdownElementBuilder {
+ @override
+ Widget? visitElementAfter(md.Element element, TextStyle? preferredStyle) {
+ final String textContent = element.textContent;
+ final bool isMultilineCode = textContent.contains('\n');
Review Comment:
Can we check it the element is in single backticks and triple backticks
instead of looking for `\n`? There may be single lines formatted as codeblocks.
I guess they will still contain `\n` at the end, but the number of ticks is an
immediate indicator and so it feels more reliable.
##########
learning/tour-of-beam/frontend/lib/pages/tour/widgets/unit_content.dart:
##########
@@ -30,6 +32,76 @@ class UnitContentWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
- return MarkdownBody(data: unitContent.description);
+ final textTheme = Theme.of(context).textTheme;
+ final markdownStyle = MarkdownStyleSheet(
Review Comment:
Let's add `final MarkdownStyleSheet markdownStyleSheet` to
`BeamThemeExtension`. This will simplify the dependencies on the theme below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
