-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GSoC] Add Menu Component, Sidebar Component and Floating Action Button. #2299
base: ucr
Are you sure you want to change the base?
Conversation
Change-Id: I4cb7bb717b5be11b6ba5ee5051e2a7f03fcb48f7
Change-Id: If02798124ff1ffed56b744131b1ed757c0522993
Change-Id: Ieec6064bdd863e6c2deba16f2c91ea6a421e54b8
I've been experimenting with this, and the FAB is bothering me a bit for two reasons.
versus on the phone: It would be good if we could float it similarly. Also note that in the app on the phone I get a back arrow rather than a hamburger menu. I'm not sure what's causing that. |
@AppInventorWorkerBee ok to test |
@singhalsara8 That doesn't seem to fix the issue. I don't think the issue is with the FAB's z-index (999 was likely fine), but rather that it's size is considered greater than 0 for the purposes of computing other element sizes in the MockForm's vertical arrangement. The easiest way to see this is to add any element and set its width and height to Fill Parent. Before this change, the component will fill the MockForm. After this change, it will only fill up to the FAB. |
Also, I don't think the libraries are set up correctly for compiling apps. When I build an empty project and attempt to start it in the emulator I get: E/AndroidRuntime( 2458): java.lang.RuntimeException: Unable to start activity ComponentInfo{appinventor.ai_ewpatton.BlankProject/appinventor.ai_ewpatton.BlankProject.Screen1}: java.lang.RuntimeException: Failed to resolve attribute at index 3
E/AndroidRuntime( 2458): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2325)
E/AndroidRuntime( 2458): at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387)
E/AndroidRuntime( 2458): at android.app.ActivityThread.access$800(ActivityThread.java:151)
E/AndroidRuntime( 2458): at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303)
E/AndroidRuntime( 2458): at android.os.Handler.dispatchMessage(Handler.java:102)
E/AndroidRuntime( 2458): at android.os.Looper.loop(Looper.java:135)
E/AndroidRuntime( 2458): at android.app.ActivityThread.main(ActivityThread.java:5254)
E/AndroidRuntime( 2458): at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime( 2458): at java.lang.reflect.Method.invoke(Method.java:372)
E/AndroidRuntime( 2458): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
E/AndroidRuntime( 2458): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)
E/AndroidRuntime( 2458): Caused by: java.lang.RuntimeException: Failed to resolve attribute at index 3
E/AndroidRuntime( 2458): at android.content.res.TypedArray.getColorStateList(TypedArray.java:425)
E/AndroidRuntime( 2458): at com.google.android.material.resources.MaterialResources.getColorStateList(MaterialResources.java:71)
E/AndroidRuntime( 2458): at com.google.android.material.floatingactionbutton.FloatingActionButton.<init>(FloatingActionButton.java:221)
E/AndroidRuntime( 2458): at com.google.android.material.floatingactionbutton.FloatingActionButton.<init>(FloatingActionButton.java:200)
E/AndroidRuntime( 2458): at com.google.android.material.floatingactionbutton.FloatingActionButton.<init>(FloatingActionButton.java:196)
E/AndroidRuntime( 2458): at com.google.appinventor.components.runtime.AppInventorCompatActivity.onCreate(AppInventorCompatActivity.java:100)
E/AndroidRuntime( 2458): at com.google.appinventor.components.runtime.Form.onCreate(Form.java:309)
E/AndroidRuntime( 2458): at appinventor.ai_ewpatton.BlankProject.Screen1.onCreate(Screen1.yail:10003)
E/AndroidRuntime( 2458): at android.app.Activity.performCreate(Activity.java:5990)
E/AndroidRuntime( 2458): at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1106)
E/AndroidRuntime( 2458): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2278)
E/AndroidRuntime( 2458): ... 10 more
W/ActivityManager( 1518): Force finishing activity 1 appinventor.ai_ewpatton.BlankProject/.Screen1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @singhalsara8, this is an impressive body of work. I've identified a number of improvements that could be made to the code. You will also want to reset your lib/blockly to match master. Assuming that you have mit-cml as a remote called upstream
, you should be able to do the following:
git fetch upstream
git checkout upstream/master lib/blockly
git submodule update lib/blockly
git add lib/blocky
git commit -m "Reset lib/blockly to master"
@@ -360,12 +360,12 @@ public void rename() { | |||
|
|||
@Override | |||
public boolean canDelete() { | |||
return !isForm(); | |||
return !isForm() && !isSidebar() && !isSidebarHeader(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather people be allowed to start a project without the sidebar and add it when they need it rather than force it to always be present. This would mean they should also have the ability to remove it. We also need this to be true of the FAB, which can be "deleted" but if you refresh the page it comes back
@@ -380,7 +380,7 @@ public void delete() { | |||
properties.addPropertyChangeListener(this); | |||
|
|||
// Allow dragging this component in a drag-and-drop action if this is not the root form | |||
if (!isForm()) { | |||
if (!isForm() && !isMenu() && !isSidebar()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a more generic approach, add a isDraggable
method to MockComponent
that returns true
by default and then have MockForm, MockMenu, and MockSidebar (and probably MockFAB) return false
@@ -0,0 +1,51 @@ | |||
package com.google.appinventor.client.editor.simple.components; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add missing copyright preamble.
- Indentation should be 2 spaces, not 4.
* @author singhalsara48@gmail.com (Sara Singhal) | ||
*/ | ||
@DesignerComponent(version = YaVersion.SIDEBAR_HEADER_COMPONENT_VERSION, | ||
description = "Container to contain components for Sidebar Header", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation for these continuation lines should be 4 spaces.
import com.google.appinventor.components.common.YaVersion; | ||
|
||
/** | ||
* Container to contain components for Sidebar Header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description is used for the documentation, so you might want to expand on the text to explain the purpose of the sidebar, etc.
@@ -2393,6 +2397,20 @@ private void showAboutApplicationNotification() { | |||
Notifier.oneButtonAlert(this, message, title, buttonText); | |||
} | |||
|
|||
public void getComponent(View component,ContextMenu menu){ | |||
componentMap.put(component,menu); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space after ,
} | ||
|
||
@Override | ||
public void onCreateContextMenu(android.view.ContextMenu menu, View v, android.view.ContextMenu.ContextMenuInfo menuInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can import android.view.ContextMenu.ContextMenuInfo
and not have to use it fully qualified here.
public void onCreateContextMenu(android.view.ContextMenu menu, View v, android.view.ContextMenu.ContextMenuInfo menuInfo) { | ||
super.onCreateContextMenu(menu, v, menuInfo); | ||
for (View.OnCreateContextMenuListener listener: onCreateContextMenuListeners) { | ||
if(listener.equals(componentMap.get(v))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space between if
and (
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
@DesignerComponent(version = YaVersion.MENU_COMPONENT_VERSION, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a Javadoc to describe more fully what this class does and how it is used. This is used to create the user facing documentation so you'll want it to be easily understood by non-programmers. Once you build the system, it will update docs/markdown/components/layout.md and docs/html/components/layout.html based on what you put here.
public void onCreateOptionsMenu(android.view.Menu menu) { | ||
this.menu = menu; | ||
for (MenuItem item : items) { | ||
Log.d(LOG_TAG, "Adding menu item"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could remove logging statements in this file.
The tests are also failing, but this is due to the presence of the FAB by default in the Form. I think we shouldn't have sidebar and FAB turned on by default and rather that they should be enabled by dragging the component into the form to turn it on. If they aren't initialized by default, I believe the tests should pass. |
I'm still getting this error on the older emulator running SDK 7: E/dalvikvm( 237): Could not find class 'com.google.android.material.navigation.NavigationView', referenced from method com.google.appinventor.components.runtime.AppInventorCompatActivity.getSidebar It seems like we may need to punt on the Sidebar if the SDK is too old, similar to FAB. Check to see whether the SDK is < 14 before instantiating the sidebar view. |
Also, I'm wondering if we should make it so that if the user selects the MockForm we close any open menus/sidebars. If you select these items in classic mode they open, but then there is no way to close them. Selecting the form would act as a "cancellation" intent. |
Change-Id: I9502cfbcdc9eae7e6188ee3c9562ed2817a34c0e
I have added three components: