Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Feature/filter fixes #387

Merged
merged 6 commits into from
Jul 30, 2015
Merged

Feature/filter fixes #387

merged 6 commits into from
Jul 30, 2015

Conversation

noahmanger
Copy link
Contributor

This does a few things to clean up the filters:

  1. Groups /disbursements filters into accordions (like the /receipts filters)
  2. Adds the date filters to /disbursements and moves those fields to a macro
  3. Positions the "remove" button inside text inputs
  4. Various other little style cleanups (the sidebar / filter code is getting gnarly and is probably due for a refactor soon)

I imagine the stuff I did to dates is going to conflict with #386, so let's merge that first and then I'll make the appropriate changes.

padding: 0 1rem;
}

.accordion__item {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason this wouldn't be put in accordion.scss. Putting it here makes it harder to find.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Moved to _side-panel.scss along with other modifications to the accordion components from being within .side-panel.

</ul>
<div class="date-range-input">
<label for="min_date">From</label>
<input type="text" id="min_date" name="min_date" data-inputmask="'alias': 'mm/dd/yyyy'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there code that uses this inputmask?

When doing research for the pattern library, we found traditional input mask code can be problematic for screen readers: https://www.filamentgroup.com/lab/politespace.html

If we're using a traditional input mask, I suggest a discussion with Bristow to check, I would be fine merging this before that happens though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Just saw that discussion. Yeah, we're currently using it, but I'll file an issue to investigate further.

@jmcarp jmcarp merged commit 2476267 into develop Jul 30, 2015
@jmcarp
Copy link
Contributor

jmcarp commented Jul 30, 2015

Added a few fixes / generalizations post-merge in cf0be5e and 0c9cfa1.

@noahmanger noahmanger deleted the feature/filter-fixes branch August 3, 2015 22:47
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.

None yet

3 participants