Skip to content
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

Improvements for Admin::Budget::Investment filters #2344

Merged

Conversation

aitbw
Copy link
Collaborator

@aitbw aitbw commented Jan 19, 2018

Where

What

  • Simplify the UI/UX of the Admin::Budget::Investment index view when the user wants to filter shown records

How

  • Added feasible, selected, undecided, unfeasible and max_supports_per_heading filters
  • Removed filters based on mockup found on Improve investment's filters #1624
  • Added without_valuator and under_valuation tabbed filters
  • Added missing tbody HTML tag in investments partial
  • Replaced selection header on investments partial with selected
  • All Admin::Budget::Investment filter-related failing specs were adapted to fit the new UI
  • Added spec for max_supports_per_heading filter (Taken from Ayuntamiento de Madrid)
  • Added partial to render a currently applied filters message to the user

Screenshots

peek 2018-01-22 08-48

Test

  • Increased coverage for the max_supports_per_heading filter, fixed coverage as necessary

Warnings

  • These specs were commented for this PR since Improve investment's filters #1624 (as shown on the mockup and the Pending section) are, well, pending. Awaiting further requirements to re-enable the #winners filter tab

  • The filters-used combination description is the only thing missing ATM

@aitbw aitbw force-pushed the aperez-investments-filters branch 4 times, most recently from 4333b79 to 0a80a66 Compare January 19, 2018 16:27
@aitbw aitbw changed the title Improved Admin::Budget::Investment filters Improvements for Admin::Budget::Investment filters Jan 19, 2018
@aitbw aitbw force-pushed the aperez-investments-filters branch 2 times, most recently from dca61ea to 4646270 Compare January 19, 2018 19:36
Copy link
Contributor

@MariaCheca MariaCheca left a comment

Choose a reason for hiding this comment

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

Looks good! Otherwise, as these changes concern files related to the frontend I'm missing some screen shots of the result to have more context, could you add them to the PR @aitbw? Thanks a lot! 🤗

@@ -15,6 +15,7 @@ class Admin::BudgetInvestmentsController < Admin::BaseController
def index
respond_to do |format|
format.html
format.js { render layout: false }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So we can make use of the advanced filters without reloading the entire page and just update what's needed —see GIF above.

@MariaCheca MariaCheca merged commit 1c2949b into consuldemocracy:master Jan 23, 2018
@aitbw aitbw deleted the aperez-investments-filters branch January 23, 2018 22:09
clairezed pushed a commit to CDJ11/CDJ that referenced this pull request Jun 26, 2018
…tments-filters

Improvements for Admin::Budget::Investment filters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve investment's filters
2 participants