Gutenberg/dark mode integration by marecar3 · Pull Request #11004 · wordpress-mobile/WordPress-Android

marecar3 · 2019-12-21T21:56:18Z

Fixes implements Dark Mode on Android platform: wordpress-mobile/gutenberg-mobile#1675

To test: Instructions can be found on the following Gutenberg PR: wordpress-mobile/gutenberg-mobile#1717

PR submission checklist:

I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2019-12-21T22:07:13Z

You can test the changes on this Pull Request by downloading the APK here.

Remove unused Log

…ation

khaykov

Thanks for the PR, @marecar3 ! I have a couple of comments inline :)

khaykov · 2020-01-08T19:39:06Z

RELEASE-NOTES.txt

@@ -1,3 +1,7 @@
+14.1
+-----
+* Block Editor: Add support for Dark Mode


Since this branch targets feature branch, it's probably not going to get released just yet :)

khaykov · 2020-01-08T19:45:19Z

...itor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java

        return view;
    }

+    private String getAppTheme() {


I'm not really sure about the details of GB implementation, but this approach of getting string resource from the app_theme_entry_value_* looks a bit hacky to me. Can GB be aware of the current theme from withing its own code? Or we could just use boolean to indicate light or dark mode.

Fixed. Changed to boolean.

khaykov · 2020-01-08T19:48:25Z

libs/editor/WordPressEditor/src/main/res/values-night/colors.xml

+<resources>
+
+    <!-- Gutenberg -->
+    <color name="background_color">#121212</color>


While overriding color values is a legit approach, ideally we would use attributes for this, that we could override in styles.xml on client-side. If GB would use other standard material color attributes (colorPrimary, colorSecondary,colorBackground, etc.), styling it from the client would also be a breeze.

PS: Discregard this.

khaykov · 2020-01-08T23:09:06Z

...r/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergContainerFragment.java

                localeString,
-                translations);
+                translations,
+                getContext().getResources().getColor(R.color.background_color));


We have defined background color in our styles.xml for both light and dark mode, so I think we can use an existing color attribute here, like this:
ContextExtensionsKt.getColorFromAttribute(getContext(), R.attr.backgroundColor)

ContextExtensionsKt isn't available in this class. Not sure if we want to include it?

# Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/PostsListActivity.kt # libs/gutenberg-mobile

…evelop Merge 14.3 final into develop

mchowning · 2020-03-09T21:04:48Z

This is looking good.

Connect a Site Screen

I did notice a weird issue when switching battery mode on/off from the editor. It would load up the connect a site screen.

UPDATE: This happened to me a number of times, but it also stopped occurring with me shortly after I originally posted this comment, so it may have just been a temporary local issue.

I only tested this using an emulator because I don't have an API<29 device handy.

App Settings appear to be disabled

Also, and this isn't an issue with the gutenberg changes, but to my eyes, I found that the coloring of the options on the app settings screen made it seem like all the options other than privacy settings were disabled.

I realize this code has been sitting for awhile (so sorry about the slow review 😞) so maybe these are just the result of not having the latest code on this branch? cc: @khaykov

khaykov · 2020-03-09T21:17:28Z

@mchowning Yeah, this looks like an old version of the feature branch :) I'll check out the Connect a Site Screen things, but it's not related to this PR.

oguzkocer · 2020-03-09T21:49:45Z

We're freezing 14.4 today, so this PR is being bumped to 14.5. If you'd like it to ship with 14.4, please merge it into release/14.4 and ping me – I'll be happy to cut a new beta release.

Update Nokogiri to 1.10.9 to fix a warning

Updates FluxC version to 1.6.7

…ation # Conflicts: # libs/gutenberg-mobile

…rt_configuration_change Fix QuickStart dialog configuration change issue

…rowser_custom_domain Open blogs in external browser with custom domains

…ation

marecar3 · 2020-03-13T12:58:06Z

WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

        setTitle(SiteUtils.getSiteNameOrHomeURL(mSite));
        mSectionsPagerAdapter = new SectionsPagerAdapter(fragmentManager);

+        setupViewPager();


Hey @khaykov,
I moved the code into a separate method as our changes have exceeded the allowed number of lines per method which resulted in failed lint tests.

marecar3 · 2020-03-13T12:58:19Z

WordPress/src/main/res/layout-w528dp/insights_management_fragment.xml

        tools:listitem="@layout/insights_management_list_item" />

-</ScrollView>
+</ScrollView>


Fixed lint error.

marecar3 · 2020-03-13T12:58:26Z

WordPress/src/main/res/layout/insights_management_fragment.xml

        tools:listitem="@layout/insights_management_list_item" />

-</ScrollView>
+</ScrollView>


Fixed lint error.

khaykov

Works like a charm 👍 Thank you so much for great work, @marecar3 !

marecar3 added 5 commits December 20, 2019 17:48

Merge branch 'develop' into feature/material-theme

87a5021

Update gb-mobile reference

74ad263

return current app theme

f54df20

Update gb-mobile reference

d20fd42

Update release notes

09962ce

marecar3 mentioned this pull request Dec 21, 2019

Feature/1675 dark mode android integration wordpress-mobile/gutenberg-mobile#1717

Merged

3 tasks

marecar3 added 3 commits December 21, 2019 23:17

Update gb-mobile reference

ea4682a

Update gb-mobile ref

eff9aa9

Update gb-mobile reference

0b5bd03

Remove unused Log

marecar3 marked this pull request as ready for review December 25, 2019 16:25

marecar3 requested review from cameronvoell, hypest, maxme and mchowning December 25, 2019 16:25

marecar3 added the Gutenberg Editing and display of Gutenberg blocks. label Dec 25, 2019

marecar3 requested a review from khaykov December 25, 2019 16:26

Merge branch 'feature/material-theme' into gutenberg/dark_mode_integr…

fd91a12

…ation

marecar3 added the Dark Mode label Dec 25, 2019

Prepare and set default background color for react root view

e329aa8

khaykov self-assigned this Jan 7, 2020

marecar3 added 2 commits January 8, 2020 11:23

Merge branch 'feature/material-theme' into gutenberg/dark_mode_integr…

7e1e869

…ation

Update gb-mobile reference

7b4ccd1

khaykov reviewed Jan 8, 2020

View reviewed changes

marecar3 added 5 commits January 13, 2020 18:39

Merge branch 'develop' into gutenberg/dark_mode_integration

562d96c

# Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/PostsListActivity.kt # libs/gutenberg-mobile

Use event instead of callback for preferred color scheme

c9fa076

update gb-mobile reference

13b2905

update gb-mobile reference

27c2128

fix linter

81325bb

Merge pull request #11414 from wordpress-mobile/merge/14.3-final-to-d…

74413a3

…evelop Merge 14.3 final into develop

oguzkocer modified the milestones: 14.4, 14.5 Mar 9, 2020

oguzkocer and others added 17 commits March 9, 2020 18:01

Updates FluxC version to 1.6.7

0f8506c

Merge pull request #11417 from wordpress-mobile/update/nokogiri

09da08b

Update Nokogiri to 1.10.9 to fix a warning

Merge pull request #11418 from wordpress-mobile/update/fluxc-version

0f7f9cf

Updates FluxC version to 1.6.7

reattach fragment listeners on resume.

d063aae

external browser opens with custom domain and no preview params

29d4333

Merge branch 'feature/material-theme' into gutenberg/dark_mode_integr…

c58f6f4

…ation # Conflicts: # libs/gutenberg-mobile

Update gb-mobile reference

3e18c88

Merge pull request #11419 from wordpress-mobile/issue-10845/quick_sta…

e7f1453

…rt_configuration_change Fix QuickStart dialog configuration change issue

Merge pull request #11423 from wordpress-mobile/issue-8884/external_b…

297e677

…rowser_custom_domain Open blogs in external browser with custom domains

Update gb-mobile reference

4339310

Merge branch 'develop' into gutenberg/dark_mode_integration

752f90b

revert merge with develop

16efd2d

Update gb-mobile reference

b8a2860

Merge branch 'feature/material-theme' into gutenberg/dark_mode_integr…

875d4fd

…ation

Pass is dark mode state when fragment is recreated

9284b0e

Remove release notes as we will do it later

fc1322e

Fix lint errors

0963c4e

marecar3 commented Mar 13, 2020

View reviewed changes

khaykov approved these changes Mar 13, 2020

View reviewed changes

khaykov merged commit 6357643 into feature/material-theme Mar 13, 2020

khaykov deleted the gutenberg/dark_mode_integration branch March 13, 2020 19:28

marecar3 mentioned this pull request Mar 25, 2020

Feature/1675 dark mode android integration wordpress-mobile/gutenberg-mobile#2061

Closed

3 tasks

Conversation

marecar3 commented Dec 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Dec 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khaykov 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!

khaykov Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mchowning commented Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Connect a Site Screen

App Settings appear to be disabled

Uh oh!

khaykov commented Mar 9, 2020

Uh oh!

oguzkocer commented Mar 9, 2020

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!

khaykov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

marecar3 commented Dec 21, 2019 •

edited

Loading

peril-wordpress-mobile bot commented Dec 21, 2019 •

edited

Loading

khaykov Jan 8, 2020 •

edited

Loading

mchowning commented Mar 9, 2020 •

edited

Loading