FlutterKaigi / conference-app-2023

The Official Conference Application for FlutterKaigi 2023
https://flutterkaigi.jp/conference-app-2023/
Apache License 2.0
132 stars 27 forks source link

Add `SelectionArea` #143

Closed takassh closed 1 year ago

takassh commented 1 year ago

Description

Wrap RootScreen with SelectionArea to be able to select texts.

Fixes #135

Type of change

How Has This Been Tested?

Check if text can be select or not on screens.

Checklist:

koji-1009 commented 1 year ago

@takassh Excuse me. I made a mistake.

With the current implementation, it seems that selection cannot be made on non-root screens such as licenses/about-us. Could you investigate this issue?

takassh commented 1 year ago

@koji-1009 It seems SelectionArea uses GestureDetector to fire selection, so after you move screen, SelectionArea is not the ancestor of AboutUsPage. See image.

Screenshot 2023-09-12 at 10 46 22

However, we use ShellRoute and it includes other routes in its tree. So, we can select texts, for example,HomePage.

Screenshot 2023-09-12 at 10 47 53

This forces when we add more pages outside of SelectionArea tree, we should wrap the page.

How do you think?

koji-1009 commented 1 year ago

@takassh I have identified the problem. Indeed, it does not seem to work as document. How about creating a Widget with Scaffold + SelectionArea as children and replace the Scaffold in the app?

class SelectionAreaScaffold extends StatelessWidget {
  const SelectionScaffold({
    super.key,
    this.appBar,
    this.body,
  });

  final PreferredSizeWidget? appBar;
  final Widget? body;

  @override
  Widget build(BuildContext context) {
    return SelectionArea(
      child: Scaffold(
        appBar: appBar,
        body: body,
      ),
    );
  }
}
takassh commented 1 year ago

@koji-1009 sure! You mean replace all Scaffold in the app, right?

koji-1009 commented 1 year ago

@takassh YES! That's right!

takassh commented 1 year ago

@koji-1009 I updated from your suggestion. I didn't wrap licenses/legal-notices with it because it looks we don't need to copy and paste on that page. Other than the page, copy and paste are working including licenses/about-us.

github-actions[bot] commented 1 year ago

Commented by GitHub Bot

PR: #143 preview link