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

Vueify/new show #4267

Merged
merged 18 commits into from
Jun 13, 2018
Merged

Vueify/new show #4267

merged 18 commits into from
Jun 13, 2018

Conversation

sharkykh
Copy link
Contributor

@sharkykh sharkykh commented May 28, 2018

Depends on ✔️ #4059 + ✔️ #4283


There are some commits that don't belong in this branch. I will clean it when the time comes :)

Showing off the new UI 😉 (gif is a bit outdated though)

GIF

2018-05-24_03-03-01

@sharkykh sharkykh self-assigned this May 28, 2018
@OmgImAlexis OmgImAlexis mentioned this pull request May 28, 2018
@sharkykh sharkykh force-pushed the vueify/new-show branch 2 times, most recently from 11f81a0 to 457d47e Compare May 28, 2018 18:07
const { data } = response;
const { result, message, redirect, params } = data;

if (message.length !== 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this isn't an array use if (message) {}. Same goes for the redirect.length below.

@OmgImAlexis OmgImAlexis added this to the 0.2.6 milestone May 29, 2018
sharkykh added a commit that referenced this pull request Jun 2, 2018
medariox pushed a commit that referenced this pull request Jun 7, 2018
)

* Make addShows/addNewShow return a redirection url instead of redirecting

Backported from #4267 and tweaked

* Fix JS
@sharkykh sharkykh removed this from the 0.2.6 milestone Jun 10, 2018
@OmgImAlexis
Copy link
Collaborator

@sharkykh is this going to be ready for 0.2.6?

@sharkykh
Copy link
Contributor Author

sharkykh commented Jun 13, 2018

@OmgImAlexis It depends on how far I'm willing to go with this.
It's already pretty good, I just have some ideas I can most certainly do in another PR.

sharkykh added 6 commits June 13, 2018 04:07
* New search UI
* Move data endpoints to /api/v2/internal
Use axios with request cancellation for the show search.
* Unify responses:
   No redirects, return either a rendered template, or a JSON response
* Unify responses:
  No redirects, return either a rendered template, or a JSON response
* Fixed a bug where if you supplied only the show's directory it would
get added to the list of shows to add twice.
* Unify responses:
  No redirects, return either a rendered template, or a JSON response
@sharkykh sharkykh added this to the 0.2.6 milestone Jun 13, 2018
OmgImAlexis
OmgImAlexis previously approved these changes Jun 13, 2018
Copy link
Collaborator

@OmgImAlexis OmgImAlexis left a comment

Choose a reason for hiding this comment

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

LGTM

search_terms = [query]

# If search term ends with what looks like a year, enclose it in ()
matches = re.match(r'^(.+ |)([12][0-9]{3})$', query)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can probably add this as another PR but we also need to do vs and vs. as I found with Red vs. Blue.

<script src="js/vue-submit-form.js"></script>
## <script src="js/lib/[email protected]"></script>
## <script src="js/lib/vue-frisbee.min.js"></script>
## <script src="js/vue-submit-form.js"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be removed before merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I keep the files though?

Copy link
Collaborator

Choose a reason for hiding this comment

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

They're used in other files right? I was mainly talking about the python comment.

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 well, no, they're unused.
frisbee (vue-frisbee) is only used in vue-submit-form, and vue-submit-form was only used in newShow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If they're unused remove the libs as we can always add them back.

</form>
<br>
<br />
Copy link
Collaborator

Choose a reason for hiding this comment

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

The other style is preferred in HTML5.

OmgImAlexis
OmgImAlexis previously approved these changes Jun 13, 2018
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.

2 participants