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

Search components migration #880

Merged
merged 8 commits into from May 15, 2017
Merged

Search components migration #880

merged 8 commits into from May 15, 2017

Conversation

noirbizarre
Copy link
Contributor

This PR is part of #526:

  • Pure Vue.js search facets.
  • Drop some unused dependencies (sliders)
  • Use the right bundle in search/listing views
  • Improve and fix the datepicker:

},
onFocus(e) {
if (!this.picking || e.target !== this.pickedField) {
console.log('should focus');
Copy link
Member

Choose a reason for hiding this comment

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

Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

onChange(e) {
try {
this.currentValue = moment(e.target.value, DATE_FORMAT);
} catch(e) {}
Copy link
Member

Choose a reason for hiding this comment

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

Notify somehow?

onChange(e) {
try {
this.value = moment(e.target.value, this.dateFormat);
} catch(e) {}
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

onChange(e) {
try {
this.currentValue = moment(e.target.value, this.dateFormat);
} catch(e) {}
Copy link
Member

Choose a reason for hiding this comment

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

And here :)


// Legacy depdencies soon to be dropped
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😉

{% set total=territories|length %}
{% if total==1 %}
<a href="{{ territories.0.url }}">
{{ _('Territory:') }}
<span class="badge">{{ territories.0.name }}</span>
</a>
{% else %}
<a href="#territories" data-toggle="tab">
<a href@click.prevent="setTab('territories')">
Copy link
Member

Choose a reason for hiding this comment

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

Missing space before @?

.year(this.currentYear)
.format('MMMM YYYY');
monthDisplay() {
return moment().month(this.currentMonth).year(this.currentYear).format('MMMM YYYY');
Copy link
Member

Choose a reason for hiding this comment

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

Turn this format into a constant?

border: none;
border-radius: @border-radius-base;

&:hover {
Copy link
Member

Choose a reason for hiding this comment

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

Weird indent here and below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed: was mix of spaces and tabs (copy-paste :/). Now it's space everywhere

@@ -34,6 +34,7 @@
<script>
import Calendar from 'components/calendar.vue';
import moment from 'moment';
import log from 'logger';
Copy link
Member

Choose a reason for hiding this comment

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

I can't see where log is finally in use, 👍 for comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oups, forgot to remove (I started with logging first and this how I remember why the catch blocks where empty)

@@ -48,6 +48,7 @@
import moment from 'moment';

const VIEWS = ['days', 'months', 'years'];
const MONTH_FORMAT = 'MMMM YYYY';
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should put all date-related constants somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but in this this case this very specific to the way its used/displayed into the widget

@noirbizarre noirbizarre merged commit 5468020 into opendatateam:dev May 15, 2017
@noirbizarre noirbizarre deleted the search-components-migration branch May 15, 2017 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants