google / blockly-android

Blockly for Android
Apache License 2.0
671 stars 209 forks source link

Adding AbstractBlocklyFragment and demo SimpleFragment #647

Closed AnmAtAnm closed 7 years ago

AnmAtAnm commented 7 years ago

Also clarifying comments about the save path overrides.


This change is Reviewable

AnmAtAnm commented 7 years ago

Review status: 0 of 14 files reviewed at latest revision, 3 unresolved discussions.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 1 at r1 (raw file):

Previously, RoboErikG wrote…
copyright

Done.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 32 at r1 (raw file):

Previously, RoboErikG wrote…
Missing class doc

Done.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 83 at r1 (raw file):

Previously, RoboErikG wrote…
add "using {@link #getWorkspaceSavePath()}" to match comment above.

Done. Here and in AbstractBlocklyActivity.


Comments from Reviewable

RoboErikG commented 7 years ago

Review status: 0 of 14 files reviewed at latest revision, 6 unresolved discussions.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 66 at r2 (raw file):

 * bundle.  If no workspace is loaded, it defers to {@link #onLoadInitialWorkspace}.
 * <p/>
 * Configure the workspace by providing definitions for {@link #getBlockDefinitionsJsonPaths()},

nit: replace comma with ' and'


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 250 at r2 (raw file):

    }

    public boolean onOptionsItemSelected(MenuItem item) {

missing method javadoc


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 256 at r2 (raw file):

        int id = item.getItemId();

        if (id == R.id.action_save) {

Should this be a switch instead of an if/else?


Comments from Reviewable

AnmAtAnm commented 7 years ago

Review status: 0 of 14 files reviewed at latest revision, 6 unresolved discussions.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 66 at r2 (raw file):

Previously, RoboErikG wrote…
nit: replace comma with ' and'

Fixed here and in AbstractBlocklyActivity


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 250 at r2 (raw file):

Previously, RoboErikG wrote…
missing method javadoc

Done.


blocklylib-core/src/main/java/com/google/blockly/android/AbstractBlocklyFragment.java, line 256 at r2 (raw file):

Previously, RoboErikG wrote…
Should this be a switch instead of an if/else?

"Resource IDs cannot be used in switch statement in Android library modules."


Comments from Reviewable