[Tour of Beam]: Welcome Screen frontend layout #22794

nausharipov · 2022-08-19T13:47:47Z

Resolves #22583

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

* theme setup * Replaced ThemeProvider with ThemeSwitchNotifier * header with theme mode switcher and logo * page container with header & footer * theme mode tests * renamed the directory to tour-of-beam * compressed beam_logo.png * added missing license comments * rudimentary layout of the first screen * review comments fixes #1 * moved notifyListeners inside then * responsive todo * split into 2 simple functions * deleted redundant constants & replaced 2018 text theme with 2021 * styling refinement * home screen layout * clickable sign in text * font weights fix * removed _getBaseFontTheme function * fixed border and bg color * color fixes * difficulty component * _LastModuleBody * todo in test * footer border * fixed overflows * replaced Project prefix with Tob * replaced then with await * inferred type * started translation of the home screen * sorted translations * Complexity comments * comment fixes * home screen translations * sign in overlay * import fix * integration test does not fail * playground_components package with dismissible_overlay * missing license * removed _dots from build * widgets refinement * renamed home screen to welcome screen * deleted copyWith * _SdkButton * trailing comma & pubspec formatting * license and lints * license * removed license from .metadata * pubspec formatting * total lints update * changed from tour_of_beam to tour-of-beam in build.gradle.kts * license check * _SdkButton mimics Radio button * renamed MyApp to TourOfBeamApp * onChanged mimics Radio button Co-authored-by: darkhan.nausharipov <darkhan.nausharipov@kzn.akvelon.com>

nausharipov · 2022-08-19T14:56:57Z

R: @damondouglas

github-actions · 2022-08-19T14:57:59Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

damondouglas

@nausharipov Thank you for submitting this PR.

damondouglas · 2022-08-23T22:14:13Z

learning/tour-of-beam/frontend/README.md

+-->
+
+# Tour of Beam
+


May we add a README at: learning/tour-of-beam/README.md that explains the purpose of Tour of Beam to someone who has never heard of it?

Then, assuming that this learning/tour-of-beam/README.md exists, may we add:

# Overview This folder holds code that supports the user interface for [Tour of Beam](../). # Requirements To develop, build and test code in this folder requires the following: - https://flutter.dev/

damondouglas · 2022-08-23T22:22:09Z

learning/tour-of-beam/frontend/lib/pages/welcome/screen.dart

+              ],
+            ),
+          ),
+          Image.asset(TobAssets.welcomeLaptop),


May we consider removing this image?

Why do you want it removed? This image is in the approved layout:
https://www.figma.com/file/gzeYx965q4DwV6ugDzp0iG/Apache-Beam-Playground?node-id=1475%3A5359

There is a problem with it however. It is cropped nicely on the mock, but looks awkward when scrolled down:

I suggest we keep it but stick to the bottom at the crop ratio from the mock so the user cannot scroll below this:

Also for some reason the border shows hard pixels, we should see how to get them smooth if we keep the image.

The image is displayed properly if the html renderer is selected. I'll update README.

Fixed the layout, keeping the cropped laptop.

damondouglas · 2022-08-23T22:25:41Z

learning/tour-of-beam/frontend/lib/pages/welcome/screen.dart

+              )
+              .toList(),
+        ),
+        ElevatedButton(


On a wide screen, for me the "Start learning" button appeared on the right side of the "Java", "Python", and "Go" buttons. Yet, when I made the width more narrow, the "Start learning" button positioned below.

May we persist the "Start learning" below the language selection buttons?

When "Start leaning" is below, it is off a module grid and is not aligned to anything, i.e. it is wider than Java and narrower than Java + Python. It is not that much of a problem, but one line looks better. So I prefer to break only when we have to.

Compare:

Also I don't think many people notice the switch as they do not normally resize splash screens like this.

damondouglas · 2022-08-23T22:26:17Z

learning/tour-of-beam/frontend/assets/translations/en.yaml

+  darkMode: 'Dark Mode'
+  lightMode: 'Light Mode'
+  privacyPolicy: 'Privacy Policy'
+  reportIssue: 'Report issue in Jira'


I think the Beam project uses GitHub issues now instead of Jira.

damondouglas · 2022-08-23T22:28:10Z

learning/tour-of-beam/frontend/lib/components/complexity.dart

+
+  static const Map<Complexity, List<Widget>> _dots = {
+    Complexity.basic: [_Dot.green, _Dot.grey, _Dot.grey],
+    Complexity.medium: [_Dot.orange, _Dot.orange, _Dot.grey],


Unless I was in dark mode, the two orange dots were not distinguishable from the grey.

Let's do 0x30808080 that fits both light and dark?

Implemented the above.

grey dot with opacity, footer link text style

alexeyinkin · 2022-08-29T09:27:37Z

learning/tour-of-beam/frontend/lib/pages/welcome/screen.dart

-              ? horizontalHalves
-              : verticalHalves,
-        ),
+        child: MediaQuery.of(context).size.width > ScreenSizes.medium


This should be a separate constant so that we do not guess which of ScreenSizes breaks the halves. Something like this:

class ScreenBreakpoints { static const twoColumns = ScreenSizes.medium; }

But this is not final. I am struggling to come up with the name that will indicate that two columns are at or wider screen, and this is not clear yet. The same problem is also with ScreenSizes.medium.

Done with the above naming.

alexeyinkin · 2022-09-01T08:32:39Z

learning/tour-of-beam/frontend/lib/pages/welcome/screen.dart

-    return ColoredBox(
+    return Container(
+      constraints: BoxConstraints(
+        minHeight: MediaQuery.of(context).size.height -


Explicit sizes should be the last resort. Can this be done with this?

For two columns:

WelcomeScreen +- PageContainer +- SingleChildScrollView +- Stack +- Row +- Left column without laptop but with blank container with the height of the laptop +- Right column +- Laptop sticked to the bottom

For one column:

WelcomeScreen +- PageContainer +- SingleChildScrollView +- Column +- Left column without laptop but with blank container +- Laptop +- Right column

Screen.Recording.2022-09-01.at.14.40.55.mov

in that case, the laptop is independent of the left column's size.

Can the Positioned contain something like this?

Row( children: [ Expanded(/* laptop */), Expanded(/* empty SizedBox */), ], ),

The two Expandeds will split the full width in half. And the laptop will occupy the full width of its half.
If it works, we may need to put this under the content so that the right half does not make the content unclickable.

the height of the laptop is completely dependent on the height of the sized box inside of the left column.

OK, let's go with MediaQuery solution for now.

alexeyinkin

LGTM (Internal review), thanks.

alexeyinkin · 2022-09-01T10:58:45Z

@damondouglas If any minor issues still stand, we still prefer this merged now and to work on them in the next PR. This is because I will next be extracting a lot of Playground internals into a package to be reused in both Playground and ToB apps. This extensive refactoring requires that there are no pending merges in the two frontentds.

damondouglas

@damccorm LGTM yet please see the comments below.

From a neurodivergent point of view, the positioning of the "Start Learning" button, vertical to the language selection buttons on a wider screen, may be slightly confusing and may exclude members of this community but I do not have any data to evaluate this hypothesis. Additionally, from a neurodivergent point of view, the image may present too much visual stimulus that distracts and fatiques said users. However, again, I have no data to evaluate this hypothesis.

damccorm

Thanks for calling that out - with that said, I think the amount of visual stimulus is pretty consistent with our website more generally and AFAIK that hasn't posed any particular issues. I'm not sure I share your concern about the positioning of the start learning button, I don't think its location is obviously less clear to me (I too lack any data to back that up 🙃).

I'm inclined to leave it, but also to try to be sensitive to any feedback we might receive about it going forward. Other than that, this LGTM so I'm going to go ahead and merge.

alexeyinkin and others added 2 commits August 19, 2022 19:34

Tour of Beam frontend blank project

3aff786

github-actions bot added build learning labels Aug 19, 2022

removed whitespace from readme (issue-22583)

fddc784

nausharipov marked this pull request as ready for review August 19, 2022 14:56

renamed "content" to "child" to mimic widgets

23f9e1b

damondouglas requested changes Aug 23, 2022

View reviewed changes

README in tour-of-beam

3a88e3d

damondouglas mentioned this pull request Aug 25, 2022

Go SDK Katas: Update beam module dependency #22753

Merged

2 tasks

darkhan.nausharipov and others added 6 commits August 25, 2022 13:35

translation path rename,

fb7b41a

grey dot with opacity, footer link text style

Merge branch 'apache:master' into issue22583_tob_welcome_screen

2ddf925

report issue in github, grey dot color

0480e9a

table instead of row to clip the laptop image

6c31d4a

horizontalHalves & verticalHalves

3e3fe35

cropped laptop image

060079c

nausharipov marked this pull request as draft August 26, 2022 13:30

nausharipov requested a review from alexeyinkin August 29, 2022 05:30

row in an expensive intrinsic height

b3842b6

alexeyinkin reviewed Aug 29, 2022

View reviewed changes

darkhan.nausharipov added 2 commits September 1, 2022 14:14

laptop image in the bottom

2ba2d40

ScreenBreakpoints

8f69df6

nausharipov requested a review from alexeyinkin September 1, 2022 08:18

alexeyinkin reviewed Sep 1, 2022

View reviewed changes

darkhan.nausharipov added 2 commits September 1, 2022 14:59

intrinsic height is not needed!

f4486b9

_WideWelcome and _NarrowWelcome

40b8a2e

nausharipov requested a review from alexeyinkin September 1, 2022 09:15

draft readme

5b38e50

darkhan.nausharipov added 2 commits September 1, 2022 15:27

blank line in readme

22d08d9

removed irrelevant info from readme

39368e3

alexeyinkin approved these changes Sep 1, 2022

View reviewed changes

removed whitespace

3605cc6

nausharipov marked this pull request as ready for review September 1, 2022 10:04

nausharipov requested a review from damondouglas September 2, 2022 03:15

damondouglas approved these changes Sep 2, 2022

View reviewed changes

damccorm approved these changes Sep 2, 2022

View reviewed changes

damccorm merged commit 9e475f6 into apache:master Sep 2, 2022

nausharipov deleted the issue22583_tob_welcome_screen branch March 28, 2023 10:31

[Tour of Beam]: Welcome Screen frontend layout #22794

[Tour of Beam]: Welcome Screen frontend layout #22794

Uh oh!

Conversation

nausharipov commented Aug 19, 2022

GitHub Actions Tests Status (on master branch)

Uh oh!

nausharipov commented Aug 19, 2022

Uh oh!

github-actions bot commented Aug 19, 2022

Uh oh!

damondouglas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexeyinkin left a comment

Choose a reason for hiding this comment

Uh oh!

alexeyinkin commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damondouglas left a comment

Choose a reason for hiding this comment

Uh oh!

damccorm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexeyinkin commented Sep 1, 2022 •

edited

Loading