Skip to content

Conversation

@ericwindmill
Copy link
Contributor

@ericwindmill ericwindmill commented Jan 8, 2026

Description of what this PR is changing or adding, and why:

This PR contains fixes to the remaining issues related to FWE lessons.

Issues fixed by this PR (if any):

PRs or commits this PR depends on (if any):

Presubmit checklist

  • If you are unwilling, or unable, to sign the CLA, even for a tiny, one-word PR, please file an issue instead of a PR.
  • If this PR is not meant to land until a future stable release, mark it as draft with an explanation.
  • This PR follows the Google Developer Documentation Style Guidelines—for example, it doesn't use i.e. or e.g., and it avoids I and we (first-person pronouns).
  • This PR uses semantic line breaks
    of 80 characters or fewer.

@gemini-code-assist
Copy link
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@flutter-website-bot
Copy link
Collaborator

flutter-website-bot commented Jan 8, 2026

Visit the preview URL for this PR (updated for commit b282673):

https://flutter-docs-prod--pr12900-ew-fwe-cleanup-4r555rqf.web.app

@ericwindmill ericwindmill mentioned this pull request Jan 8, 2026
10 tasks
Copy link
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is draft but lgtm

@ericwindmill ericwindmill marked this pull request as ready for review January 9, 2026 22:10
@ericwindmill ericwindmill requested a review from a team as a code owner January 9, 2026 22:10
@ericwindmill ericwindmill requested a review from parlough January 9, 2026 22:10
@ericwindmill ericwindmill changed the title Resolve FWE clean up tickets. [DNM] Resolve FWE clean up tickets. Jan 9, 2026
@ericwindmill ericwindmill changed the title [DNM] Resolve FWE clean up tickets. Resolve FWE clean up tickets. Jan 9, 2026
@ericwindmill ericwindmill changed the title Resolve FWE clean up tickets. FWE clean up Jan 9, 2026
Comment on lines 105 to 132
bool _isVideoEmbed(Node node) {
if (node case ElementNode(tag: final tag)) {
final lowerTag = tag.toLowerCase();
return lowerTag == 'youtubeembed' || lowerTag == 'lite-youtube';
}
return false;
}

/// Finds a video embed child in a list of nodes, returns it if it's the only
/// meaningful content (ignoring whitespace text nodes)
Node? _findVideoChild(List<Node>? children) {
if (children == null) return null;

Node? videoChild;
for (final child in children) {
if (_isVideoEmbed(child)) {
videoChild = child;
} else if (child case TextNode(:final text)) {
// Ignore whitespace-only text nodes
if (text.trim().isNotEmpty) return null;
} else {
// Non-video, non-whitespace content found
return null;
}
}
return videoChild;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was written by gemini. It feels wrong to me, but I'm not sure what the proper Jaspr way to accomplish this is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that it was automatically wrapping the Youtube component in a p tag, and that would require messy CSS

@ericwindmill
Copy link
Contributor Author

@parlough, @schultek -- I'd greatly appreciate if you can look at the code I added to tell me why its bad :) I left comments with what I think is wrong with it.

Otherwise, please look at the staged site and tell me what you think about how it looks!

Also, there are still some content edits I need to make before this lands.

@ericwindmill ericwindmill requested a review from schultek January 12, 2026 20:10
@ericwindmill ericwindmill merged commit 7e4b2de into main Jan 13, 2026
10 checks passed
@ericwindmill ericwindmill deleted the ew-fwe-cleanup branch January 13, 2026 16:34
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 this pull request may close these issues.

5 participants