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.

Sign up for GitHub

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

Jump to bottom

Feature: ui grid filtering #507

Merged
merged 3 commits into from
Jun 21, 2016
Merged

Feature: ui grid filtering #507

merged 3 commits into from
Jun 21, 2016

Conversation

jniles
Copy link
Collaborator

@jniles jniles commented Jun 17, 2016 •
edited
Loading

For full details of this change, see the commit messages. The changes are best explained via screenshots:

Screenshots
thepatientregistry
Fig 1: The Unfiltered Patient Registry

filterform
Fig 2: The New Filter Form

filteredregistry
Fig 3: The New Filter Bar

filteredpdfreport
Fig 4: Filters appear on rendered PDF report

Major Features

  1. Clicking "x" on any filter will remove it and re-query the database.
  2. The filters are cached, so that page refresh remembers the filters.
  3. The filter form will be re-populated with filters from the grid when it is opened a second time.
  4. The "Clear all Filters" button removes all filters from the grid.
  5. The dates on the form have a clearer UI (start, end).
  6. The PDF now shows what filters rendered it in a bar identical to the client's bar.

Closes #492. Closes #336.

Thank you for contributing!

Before submitting this pull request, please verify that you have:

  • Run your code through JSHint. Check out our styleguide.
  • Run integration tests.
  • Run end-to-end tests.
  • Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the official review checklist that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process and help build a stronger application. Thanks!

@jniles
Copy link
Collaborator Author

jniles commented Jun 20, 2016

@sfount, could I get a review?

sfount
sfount reviewed Jun 21, 2016
View reviewed changes
client/test/e2e/patient/registry.search.js
FU.buttons.submit();

expectNumberOfGridRows(1);
expectNumberOfFilters(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@sfount
Copy link
Contributor

sfount commented Jun 21, 2016

@jniles This LGTM - very slick and well thought through presentation on the filter bar.

I've left one note about comments on the directive, if this will go through further iteration soon just let me know - it is not at all blocking on this pull request.

@jniles
refactor(filters): improve Patient Registry filter
3f7bc03 
This commit refactors the Patient Registry filters to automatically
template into the registration page nice-looking filters.

Features landing in this commit:
 1. Filters are now cancellable, which refreshes the grid with the
 remaining filters.
 2. Filters are automatically templated into the PDF report generated by
 clicking the `print()` button on the registry page.
 3. "Clear All Filters" button clears all filters on the grid and
 refreshes the grid's data via an HTTP request
 4. Filters are cached so that a page refresh does not clear the
 filters.
 5. The filter form UI has drastically improved, allows start and end
 dates to be laid out side by side.

There are some future directions left to do after the commit.
 1. Improve rendering of filters in PDF reports.  The dates are not
 properly templated in, now do the values have much meaning.
 2. Rename the `bhFiltersApplied` directive to something a bit more
 descriptive of it's form and function.  Something like `bhFilterBar` or
 similar.
 3. Implement filtering tests as unit tests for search modal.
@jniles
feat(filters): improve filter ui rendering
45f2f44 
This commit improves the filters ui again by introducing the following
upgrades:
 1. The server uses `moment` to parse dates in the date filters.  The
 code on the server for rendering the filters is very similar to the
 clientside codebase, which is unavoidable.  There might be an
 optimisation down the road to reduce this replication, but I think this
 is acceptable for now.

 2. The filters are now passed into the filter form and preset there.
 That way, when you go to filter a second time, all the data is
 preserved.

 3. A weird bug with caching filters was fixed.  The detailed flag no
 longer shows up as a potential filter, and refreshing the page after
 printing no longer breaks the browser.

 4. The date rendering has been improved in the handlebar's date helper
 function.  If an invalid date occurs, it will simply render the empty
 string.

 5. The `$uibModal` handler now implements the behavior described in
 #506.

Closes #492.  Closes #336.
@jniles
docs(bhFiltersApplied): improve directive docs
aab37ab 
This commit adds doc comments to the bhFiltersApplied directive for
better readability and developer productivity in the future.
@jniles jniles force-pushed the feature-ui-grid-filtering branch from 88835bb to aab37ab Compare June 21, 2016 09:11
@jniles
Copy link
Collaborator Author

jniles commented Jun 21, 2016

@sfount, I've made the suggested changes.

@sfount sfount merged commit 4377625 into Third-Culture-Software:master Jun 21, 2016
@jniles jniles deleted the feature-ui-grid-filtering branch June 21, 2016 11:25
bors bot added a commit that referenced this pull request Oct 1, 2018
@bors @greenkeeper
Merge #3212
2ce8ce3 
3212: Update commitizen to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The devDependency [commitizen](https://github.com/commitizen/cz-cli) was updated from `2.10.1` to `3.0.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v3.0.0</summary>

<p>&lt;a name"3.0.0"&gt;</p>
<h2>3.0.0 (2018-10-01)</h2>
<h4>Bug Fixes</h4>
<ul>
<li>resolve linux build issues, add git exit code 128 warning (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/c3764c8c">c3764c8c</a>)</li>
<li><strong>deps:</strong>
<ul>
<li>update dependency glob to v7.1.2 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326327664" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#506" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/506">#506</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/aa824960">aa824960</a>)</li>
<li>update dependency lodash to v4.17.10 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326338773" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#507" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/507">#507</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4bda9435">4bda9435</a>)</li>
<li>update dependency find-root to v1.1.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326327598" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#505" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/505">#505</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/67a4e16a">67a4e16a</a>)</li>
<li>update dependency dedent to v0.7.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326314146" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#504" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/504">#504</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a21b674f">a21b674f</a>)</li>
<li>update dependency cz-conventional-changelog to v2.1.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326314071" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#503" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/503">#503</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/43d54de1">43d54de1</a>)</li>
</ul>
</li>
<li><strong>package:</strong> Remove opencollective post-install hook (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="356422024" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#561" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/561">#561</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c234d">a70c234d</a>)</li>
</ul>
<h4>Features</h4>
<ul>
<li>Drop support for Node.js &lt;6.x, update dependencies (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="361350875" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#566" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/566">#566</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b">a70c063b</a>)</li>
<li>support initialization with yarn. (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="349367370" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#549" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/549">#549</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5652154">d5652154</a>, closes <a href="https://urls.greenkeeper.io/commitizen/cz-cli/issues/527">#527</a>, <a href="https://urls.greenkeeper.io/commitizen/cz-cli/issues/527">#527</a>)</li>
</ul>
<h4>Breaking Changes</h4>
<ul>
<li>Older versions of Node.js are no longer supported</li>
</ul>
<p>Includes most updates with the exception of semantic-release which breaks on windows. To be investigated in a later release.<br>
(<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b">a70c063b</a>)</p>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 23 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b06dbdf41af322dab06f83bfddd69149b"><code>a70c063</code></a> <code>feat: Drop support for Node.js &lt;6.x, update dependencies (#566)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/0d76887b7853f673949cf7aee04653b90b4dda7b"><code>0d76887</code></a> <code>chore: remove sign from windows build</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/1546df4856dc8c4aa781e9e0ea755613647642ca"><code>1546df4</code></a> <code>chore: assume yarn in tests</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/c3764c8c1d2b05597a6c604c13b6a11731c47281"><code>c3764c8</code></a> <code>fix: resolve linux build issues, add git exit code 128 warning</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/f1150c83a62048831fe6d94836f52242fdc9831e"><code>f1150c8</code></a> <code>chore: Create jobs build yml</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/f0595edca921b68913e2ce2ca1bbad75abe71515"><code>f0595ed</code></a> <code>chore: add azure jobs</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4693079d0400ad1a914af86c6027f83d18cde74d"><code>4693079</code></a> <code>chore: Set up CI with Azure Pipelines</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5652154fa1bb41c1b1c84f20c620ee64ca32c78"><code>d565215</code></a> <code>feat: support initialization with yarn. fixes #527 (#549)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c234d8471af147cc9f7d9d090b6ba2192eb17"><code>a70c234</code></a> <code>fix(package): Remove opencollective post-install hook (#561)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5b8bc587770e4a7efd6a36962abaa0425931181"><code>d5b8bc5</code></a> <code>docs(readme): add npx use examples (#532)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d103b10f56121a8617ac9ac882338338417b0947"><code>d103b10</code></a> <code>chore(ci): test on Node.js 10.x (#531)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/41a921b002d1627cfedd4bb5745a27d8a47a5dfe"><code>41a921b</code></a> <code>BREAKING CHANGE: Remove Node 0.12 support (#524)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/aa8249605d45c1e95f57be7c832321212b0e8d1c"><code>aa82496</code></a> <code>fix(deps): update dependency glob to v7.1.2 (#506)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4bda94350625e54ca862cd143432de1912caea27"><code>4bda943</code></a> <code>fix(deps): update dependency lodash to v4.17.10 (#507)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/67a4e16a88c7925f3199996002ba8da37e420cff"><code>67a4e16</code></a> <code>fix(deps): update dependency find-root to v1.1.0 (#505)</code></li>
</ul>
<p>There are 23 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/commitizen/cz-cli/compare/5838ce007c1af2e71ef6ae68efce7682d59e9061...a70c063b06dbdf41af322dab06f83bfddd69149b">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jniles @sfount