Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Floating actions menu #2335

Merged
merged 21 commits into from
Jul 2, 2018

Conversation

Dominaezzz
Copy link
Contributor

image
signed-off-by: Dominic Fischer dominicfischer7@gmail.com

@Dominaezzz
Copy link
Contributor Author

#2096

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Dominaezzz ,

Thanks a lot for this PR, the result looks great. I've done several comments. I'll test the PR on a real phone in a second time

Please let me know if you have any questions

Benoît

@@ -167,6 +167,7 @@ dependencies {

// UI
implementation 'com.android.support.constraint:constraint-layout:1.1.0'
implementation 'com.getbase:floatingactionbutton:1.10.1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you credit this library in the file open_source_licenses.html please? There is already a section for Apache 2.0 license

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have credited the library as requested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

} else {
mFloatingActionButton.show();
mFloatingActionsMenu.collapse();
mFloatingActionsMenu.setVisibility(View.VISIBLE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

show() display the button with an animation. Is the animation still there with setVisibility() on a FloatingActionsMenu?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented animations.


Class menuClass = FloatingActionsMenu.class;
try {
Field normal = menuClass.getDeclaredField("mAddButtonColorNormal");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you obliged to do that? Isn't it possible using styles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Styles could be used but only for a one time setup.
Since the color changes at runtime, reflection was the easiest way.

public void onMenuCollapsed() {
touchGuard.setAlpha(0);

touchGuard.setOnClickListener(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not remove the interception of click event, so the user will be stuck :)
You also have to call setClickable(false)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still bugged. You have to call setClickable() AFTER setOnClickListener(). Look in setOnClickListener implementation

mFabCreateRoom.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
mFloatingActionsMenu.collapse();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two other cases does not call collapse(). So if it is necessary, add the 2 missing calls and if it is not, remove this one :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added missing calls.


if (null != mFloatingActionsMenu) {
// mFloatingActionButton.hide();
mFloatingActionsMenu.setVisibility(View.GONE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about FAB animation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented the animation.

@@ -1310,7 +1423,7 @@ public void run() {
* @return fab view
*/
public FloatingActionButton getFloatingActionButton() {
return mFloatingActionButton;
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This remove a feature: the FAB is automatically hidden during a few seconds when the user scroll the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gracefully swapped it out for a View. Since only the 'Top' of the button is queried for.

android:layout_width="wrap_content"
android:layout_height="wrap_content"
app:barrierDirection="bottom"
app:constraint_referenced_ids="listView_pending_callview"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Barrier is used to reference several ids. There is only one here, so I do not think this barrier is necessary. Maybe for code clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a barrier for clarity, I felt directly referencing the call view was clumsy but I've removed barrier as it is doing nothing.


if (!(fragment instanceof AbsHomeFragment) || !((AbsHomeFragment) fragment).onFabClick()) {
onFloatingButtonClick();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove onFloatingButtonClick() method then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mFabDialog member can also be removed then

@bmarty
Copy link
Member

bmarty commented Jun 5, 2018

It also fixes #1446

Dominaezzz and others added 15 commits June 8, 2018 20:16
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
…larity.

Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Set clickable to false by default and toggle when appropriate.

Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Remove worker thread usage in favour of handler with less allocations.

Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
…ting_actions_menu

# Conflicts:
#	vector/src/main/java/im/vector/activity/VectorHomeActivity.java
#	vector/src/main/res/layout/activity_home.xml
…_actions_menu

# Conflicts:
#	vector/src/main/java/im/vector/activity/VectorHomeActivity.java
@t3chguy
Copy link
Member

t3chguy commented Jun 9, 2018

@bmarty this is ready for another look
edit: it is for real this time

@t3chguy
Copy link
Member

t3chguy commented Jun 10, 2018

remaining test failure is the same as in /develop/ due to merge of new forbidden_strings_in_code without first fixing or otherwise the offending code.
e.g https://travis-ci.org/vector-im/riot-android/builds/389625128

Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
Signed-off-by: Dominic Fischer <dominicfischer7@gmail.com>
@bmarty
Copy link
Member

bmarty commented Jun 11, 2018

Hi @Dominaezzz and @t3chguy ,
Thanks for update and merge fix (and ok for Travis failure reason, my bad).
There is still one issue before I can merge this PR. The UI is still locked when the user close the menu.
Benoît

@Dominaezzz
Copy link
Contributor Author

Hey @bmarty, I've resolved the issues you mentioned.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Dominaezzz @t3chguy ,

Thank for the quick fix. Making more tests, I've noticed that the touch guard is not working on pre21, which is a bit annoying. Is it possible to do better? I've put some ideas in the inlined comments.
I've also detect a potential issue on the color used for the sub FABs, but is is maybe intended?

Also feel free to add your name in the AUTHORS.rst file and to add an entry in CHANGES.rst

Benoît


}

mFabJoinRoom.setColorNormal(secondaryColor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended to exchange primary and secondary color for the 3 small FABs (regarding the colors for the main FAB)? It is more visible on the "Contact" tab

app:layout_constraintRight_toRightOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
android:clickable="false"
android:elevation="10dp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch guard does not work on Pre-Lollipop which is a bit annoying. Maybe the touch guard can be placed before in the XML file to be displayed under the FAB menu

app:fab_labelStyle="@style/Floating_Actions_Menu"
app:backgroundTint="@color/vector_fuchsia_color"
app:borderWidth="0dp"
app:elevation="4dp">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is the desired elevation 4dp or 12dp? I think the 2 values should be identical.


// Pre-Lollipop does not support elevation so cannot have touch guard
// above bottomNavigationView.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The touch guard is not shown on PreLollipop, it is a bit annoying. If there is no elevation before API 21, I think this is not a problem to show the touchguard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only reason why I've disabled the touch guard pre-lollipop is because the touch guard shows up behind the Bottom Navigation View. No matter the order of views in the constraint layout, the Bottom Navigation View shows up on top of everything.
Would it be okay to have the Bottom Navigation View not be covered by the touch guard?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so it means that the Bottom Navigation View is able to use elevation, maybe by using app: namespace. If it is not possible to do better, yes, I think it is better to have the touch guard in all cases

@bmarty bmarty changed the base branch from develop to feature/new_fab July 2, 2018 12:32
@bmarty
Copy link
Member

bmarty commented Jul 2, 2018

Hi @Dominaezzz ,

I wish your work to be in the next release, so I merge it in a branch I own and will fix the very last issue by myself.

Thanks again for this PR

Benoît

@bmarty bmarty merged commit 1f2e335 into element-hq:feature/new_fab Jul 2, 2018
giomfo referenced this pull request in tchapgouv/tchap-android-legacy Aug 25, 2018
Update matrix-sdk.aar lib - build 1875 - Revision: ccf12449b8f09b06a7a8f501b9d7a382270b2305

Last rebase was performed June 1 2018. Here is the list of changes from vector-im/riot-android changes:

Changes in Riot 0.8.XX (2018-08-24)
===================================================

Features:
 - Manage server quota notices (#2440)

Improvements:
 - Do not ask permission to write external storage at startup (#2483)
 - Update settings icon and transparent logo for notifications and navigation drawer (#2492)
 - URL previews are no longer requested from the server when displaying URL previews is disabled (PR #2514)
 - Fix some plural and puzzle strings, and remove other unused ones (#2444)
 - Manage System Alerts in a dedicated section

Other changes:
 - Upgrade olm-sdk.aar from version 2.2.2 to version 2.3.0
 - move PieFractionView from the SDK to the client (#2525)

Bugfix:
 - Fix media sharing (#2530)
 - Fix notification sound issue in settings (#2524)
 - Disable app icon badge for "listen for event" notification (#2104)

Changes in Riot 0.8.13 (2018-08-09)
===================================================

Features:
 - Resurrect performance metrics (#2391)
 - Telemetry to report incidence of UISIs (#2330)
 - Add a previewer for previewing media before sending it into the room (#1742|#2445)
 - Implements ReplyTo feature (#2390)
 - Add auto completion for slash commands (#2384)
 - Support Room Versioning (#2441)

Improvements:
 - Update matrix-sdk.aar lib (v0.9.7).
 - Piwik: Update the way how stats are reported (#2402)
 - Improve BugReport screen: display a preview of the screenshot (#2318)
 - In the settings, move theme settings just below "language" (#2439)
 - Improve the display of the sources of the message in the dialog (#2348)
 - Improve the display of the buttons and the reason in the room preview (#2352)
 - In the flair section on settings, notify the user when he has no flair (#2430)
 - Improve GDPR consent webview management (#2491)
 - Support external keyboard to send messages for recent devices (#220, #1279)
 - When user ignores or un-ignores someone, notify that the app will restart (#2437)

Other changes:
 - Remove dependency to `android-gif-drawable` lib and use Glide to animate logo on Splashscreen (#2421)
 - Keep only Room.getState() method and remove Room.getLiveState() because they are similar (matrix-org/matrix-android-sdk#310)

Bugfix:
 - Fix issue on incoming call screen when "Do not disturb mode" is active (#2417)
 - Fix issue when selecting sound for notifications in the settings
 - Fix issue when changing device name in the settings (#2416)
 - Fix issue on verifying device, update the wording of the description message (#1067)
 - Messages with code blocks show other HTML as plain text (#2280)
 - Message with <p> was sometimes not properly formatted (#2275)
 - Fix notification issue when Riot is not started (#2451)
 - Fix Unable to add Matrix apps (#2466)
 - Riot auto joined a public room (#2472)
 - Remove last traces of Firebase analytics (#2481)
 - code blocks are escaped and therefore hard readable (#2484)
 - Restore the navigation of the back button in the public rooms preview header (#2473)
 - Fix issue on preference screen: device lists was not displayed (#2409)
 - Ensure notification has a title (#2242)

Changes in Riot 0.8.12 (2018-07-06)
===================================================

Bugfix:
 - Fix issue on vanished favorite and low priority room (#2413)

Changes in Riot 0.8.11 (2018-07-03)
===================================================

Features:
 - Re-request keys manually for encrypted events (#2319)
 - Add option to send voice message to a room, using a third application to record message.
   To enable in the Labs settings (PR #1762)

Improvements:
 - Update matrix-sdk.aar lib (v0.9.6).
 - New Floating Action Menu in Home screen (PR #2335)
 - Add spacing to device keys (#2314)
 - use apply() instead of commit() to save shared prefs (#2231)
 - Do not ring if "Do Not Disturb" is active (#1072)
 - Manage the "consent not given" error when declining a room invite

Other changes:
 - Remove "Matrix application" activation from the Lab section in the settings (#2341)

Bugfix:
 - Remove black borders on 18:9 phone (#2063)
 - Auto dismiss the join/reject room notification when user select an action (#2354)
 - Fix some crashes reported by the PlayStore (#2380, #2382, #2383, #2395)
 - Fix issues in UrlPreviews (#2312)

Translations:
 - Galician thanks to Miguel Branco

Build:
 - Add script to check code quality
 - Travis will now check if CHANGES.rst has been modified for each PR

------

Merge commit '94b7925a57b800db62b5ec87966d994ff53392b4' into develop

# Conflicts:
#	vector/src/main/java/im/vector/activity/VectorRoomActivity.java
#	vector/src/main/java/im/vector/adapters/VectorMediasViewerAdapter.java
#	vector/src/main/java/im/vector/db/VectorContentProvider.java
#	vector/src/main/java/im/vector/dialogs/ConsentNotGivenHelper.kt
#	vector/src/main/java/im/vector/fragments/VectorSettingsPreferencesFragment.kt
#	vector/src/main/res/values/strings.xml
#	vector/src/main/res/values/styles.xml
@Dominaezzz Dominaezzz deleted the floating_actions_menu branch July 2, 2019 20:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants