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

Add jquery frontend #41

Merged
merged 51 commits into from
Jan 5, 2018
Merged

Add jquery frontend #41

merged 51 commits into from
Jan 5, 2018

Conversation

jan-xyz
Copy link
Contributor

@jan-xyz jan-xyz commented Oct 20, 2017

This Pull Request adds the frontend at 127.0.0.1:5000/v2 as a single page web app. I tried to mimic the original website as much as possible in terms of elements.

fixes #24

Jan Steinke added 30 commits September 15, 2017 08:09
 I copied the layout from the existing files and begin to clean that stuff up so that it is easier to read and work with.
 This isn't fully working yet. I still have to add the actual settings to it and make it work on the initial loading of the page. Currently you have to at least select something in the box before getting boxes for the options.
 The UI loads now all necessary elements to properly send a visualize request.
 Fixes the problem that the first element has 'undefinded' prepended to the string.
 This new layout has all references to app_state removed in order to pull this information dynamically via REST from the backend.
 for now it only  uploads a single file and no information about the visualizer to use. Next step is to implement the backend for a post to visualize with a picture attached.
 It is still without the visualization settings
 The partial occlusion visualizer uses a different property than the others when delivering the image. This should be fixed in the backend.
Jan Steinke added 4 commits December 4, 2017 20:56
 By switching to blueprints for this as well I am able to use the factory pattern which is essential for Selenium testing.
 Currently the integration tests have to run first. It's weird but the tests will hang if this is executed last. That is the reason why it has the *_a_* in it. I assume there is some collision in the fixtures that causes this behavior, probably related to app shutdown.
 This is still broken in the travis file. We need to download a webdriver first.
@rhsimplex
Copy link
Contributor

Yes you can definitely use an image diff. That would be a really great unit test that would probably prevent a lot of bugs.

I defer to your judgement on how to do front end testing. When you make a decision let me know and I'll try to help you integrate it into our CI. Maybe open a new issue if you want to discuss more =)

Is this PR ready for review or are you still planning some other stuff?

@jan-xyz
Copy link
Contributor Author

jan-xyz commented Dec 5, 2017

I have a half working Selenium test in there. If you don't mind having it in master (it is currently disabled) then it's good to go. We can discuss the remainder in the new ticket (#42).

@jan-xyz
Copy link
Contributor Author

jan-xyz commented Dec 5, 2017

#43 and #44 are also two tickets I created based on this discussion. I would start working on these tickets as my next project!

@rhsimplex
Copy link
Contributor

Ok cool! I'll look through as soon as possible

 This way it doesn't interfere with the unit tests and can also be triggered separately.
@rhsimplex
Copy link
Contributor

Sorry for the delay, leaving for Christmas Monday and tons to do as usual =)

Jan Steinke added 6 commits December 31, 2017 17:13
this still doesn't check parameters.

relates to: #43
no checks of parameters

relates to: #43
This only checks one set of settings as a reference check.

relates to: #43
@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jan 1, 2018

This PR now also includes tests for #42 and #43 in a crude manner. The integration tests just check if the website builds up and the image diff tests are inspecting one set of settings and the default settings.

The integration tests are currently not part of the pipeline since I don't want to mess around with the .travis.yml too much. They can be run with pytest integration_tests if Firefox 57 and the correct geckodriver is installed. The travis file is already prepared for it and I would add the command to the script section if you permit.

@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jan 1, 2018

It looks like the reference images are exceeding the disk quota for travis docker images.

Edit: seems like this was en error on travis side not ours.

@rhsimplex
Copy link
Contributor

Interesting -- is Travis still giving you problems? Do you want me to re-review now?

@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jan 4, 2018

Ready to review :)

@rhsimplex
Copy link
Contributor

Classy stuff! I can't provide much review on the frontend files, but everything else looks good to me.

Next PR: replace the old frontend with this one??

@rhsimplex rhsimplex merged commit 3d146b8 into merantix:master Jan 5, 2018
@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jan 5, 2018

Sounds good, I'll add some more Selenium tests when doing the PR to make sure the frontend is properly tested.

@jan-xyz jan-xyz deleted the add-jquery-frontend branch February 9, 2018 19:12
@jan-xyz jan-xyz mentioned this pull request Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite web app views to use API calls
3 participants