Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Feature/kak/grid to list#15 #80

Merged

Conversation

flibbertigibbet
Copy link
Contributor

Overview

Implements the grid of buttons on the home view. Loads a random representative image for each category into the category's grid square, or hides the category's grid square if nothing matches its filter. Opens place view with filter in place.

Also fixes filters being lost on screen rotation and implements scrolling of the home view carousel with the grid content.

Demo

screenshot_1528756276

Notes

Does not attempt to ensure the images picked are unique; in practice currently, duplicates seem to be fairly uncommon.

Implements filtering by database query, as the original filtering implementation relied on string searches outside the database.

Closes #15
Fixes #78
Fixes #29

@flibbertigibbet
Copy link
Contributor Author

Although filters are now preserved on screen rotation, they are not preserved on back navigation. That might call for a separate issue.

@maurizi
Copy link
Contributor

maurizi commented Jun 12, 2018

Now that we have the ability to start a filterable activity with a filter already set, it would be good to carry the filter over when moving from events<->destinations or list<->map.

I imagine it would be pretty small to tack that onto this PR, but opening another issue to tackle it later would be fine too.

@flibbertigibbet
Copy link
Contributor Author

I don't think it will be that straightforward to manage back navigation if it's not working already with existing lifecycle events; it may require shared preferences storage. Created #81. See link to discussion on the issue.

@maurizi
Copy link
Contributor

maurizi commented Jun 12, 2018

I don't think it will be that straightforward to manage back navigation if it's not working already with existing lifecycle events;

Actually, back navigation is working fine! I meant when tapping the "map" icon it would be nice to carry the current filter into the map activity when we call startActivity by setting the current filter on the intent.

Copy link
Contributor

@maurizi maurizi left a comment

Choose a reason for hiding this comment

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

Mostly looking pretty good! Some great refactors in here 🎉
I'd like to see the "No results" layout regression fixed before merging this.

It would be nice to get the category names into string resources and to carry the filter through when going from the list -> map activities, but those can be deferred for now if you'd like.

public static final String LIKED = "Places you like";


// FIXME: reference user-facing strings (displayName) as strings resources
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to get this cleaned up as part of this PR.

Instead of a string field in the enum options, we could replace it with a @StringRes int field and leave it to the user of the enum to turn that into an actual string as-needed.

Probably fine to continue to using string constants for the dbName field.

android:gravity="center"
android:text="@string/filter_no_results"
android:visibility="gone"
app:layout_constraintTop_toBottomOf="@+id/places_list_app_bar" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't placed correctly - it shows up in the top-left hidden underneath the toolbar.

I'm guessing by the app:layout_constraintTop_toBottomOf attribute that you intended to add a ConstraintLayout to this layout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Close, I'd removed a ConstraintLayout.

@flibbertigibbet
Copy link
Contributor Author

flibbertigibbet commented Jun 12, 2018

Okay, back navigation preserves the filter, but up navigation does not. I think that should be fine. Created #81

@flibbertigibbet
Copy link
Contributor Author

Updated; this should be ready for another look.

@maurizi
Copy link
Contributor

maurizi commented Jun 13, 2018

Looks good! We should probably wait to merge this until #79 is merged.

@flibbertigibbet
Copy link
Contributor Author

I think applying #79 on top of this instead of the other way around will be easier, as it will need fix-ups for this either way, so I'd prefer to merge this first, then make a PR to fix up the merge conflicts on #79 afterwards.

Due to nested RecyclerView.
Also convert to constraint layout.
Closes azavea#29.
Get and set random image for each home view filter button category.
Save and restore filters on list view rotation.
Fixes azavea#78.
Fix last item in list being cut off/not scrolling to bottom.
Implement filtering on click of grid button on home view.
Closes azavea#15.
Also modify events list message to match formatting.
Move strings for home page fitler buttons to string resources.
@flibbertigibbet flibbertigibbet merged commit 4515fd8 into azavea:develop Jun 13, 2018
@flibbertigibbet flibbertigibbet deleted the feature/kak/grid-to-list#15 branch June 13, 2018 15:06
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.

Rotating list removes filter Make whole home page scroll Categories grid to filtered place list
3 participants