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

220 support negative facets #283

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

kghmanuel
Copy link

@kghmanuel kghmanuel commented Apr 25, 2017

Fixes #220

  1. set of changes to support negated facets
  2. requires updated ml-search-ng, drop the custom ml-search-ng.js being used.
  3. update versions of bower libraries.

Concerns:
ng-ckeditor#0.2.1 depends on angular#~1.2.9 which resolved to angular#1.2.32
ng-json-explorer#f7236fa857 depends on angular#~1.2.20 which resolved to angular#1.2.32

violation/incompatibility already existed prior to this change.
angular js version 1.5.8 chosen due to: angular-ui/ui-router#2889

2. requires updated ml-search-ng, drop the custom ml-search-ng.js being used.
3. update versions of bower libraries.
ui/app/index.ejs Outdated
@@ -42,8 +42,7 @@

<script src="bower_components/ml-common-ng/dist/ml-common-ng.min.js"></script>
<!-- fixed bug in annotateActiveFacets: -->
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can drop this comment..

Copy link
Author

Choose a reason for hiding this comment

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

deng... missed that one. will adjust

@grtjn
Copy link
Contributor

grtjn commented Apr 25, 2017

Thanks very much, this looks very promising!

I like the idea of finally dropping that local copy of ml-search-ng.js a lot.

I also like the idea of upgrading to latest angular-bootstrap. But I think you need to go over the html files to check for usage of things like (uib-)typehead, (uib-)tabset, etc. I don't recall how much we use from ui-bootstrap, but typeahead at the least. Please check against the directives listed at https://angular-ui.github.io/bootstrap/

I also think it is reasonably safe to ignore the angular dependency json-explorer. Less sure about the editor, but you could consider migrating to the editor we use in slush. Should be a relatively minor change, and pretty easy to check. By the way, don't recall where we use json-explorer. Are we actually using it somewhere?

@kghmanuel
Copy link
Author

As discussed in pull request #280, this would have to be further adjusted to have the tracked negative facets cleared on "clear()"

Gabo Manuel added 2 commits April 26, 2017 13:18
…om parent does. this makes the display accurate

2. add condition to still show entry if facet is part of active facets, i.e. all values were negated.
@kghmanuel
Copy link
Author

kghmanuel commented Apr 26, 2017

I have also reviewed the migration list: https://github.com/angular-ui/bootstrap/wiki/Migration-guide-for-prefixes. So far only the modal and modalInstance were actual hits in the previous code.

Lastly, i think migrating the rest of the items like editor should be done on a separate pull request if possible. Also not sure about json-explorer. Just reporting the warnings that gulp is telling me and that it exists prior to this pull request.

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.

2 participants