Refactor thanks pages and a little bit of the self service page #331
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,9 +32,9 @@ if (data.uuid_rider) { | |
| // Cache jQuery selectors: | ||
| var $login = $('#login'), | ||
| $manage = $('#manage'), | ||
| $info = $manage.find('#infoAdmin'), | ||
| $manageLogin = $('#manageLogin'), | ||
| $infoLogin = $manageLogin.find('#infoLogin') | ||
| ; | ||
|
|
||
|
|
||
|
|
@@ -68,25 +68,23 @@ $login.validator().on('submit', function(e) { | |
| riderExists(); | ||
| } | ||
|
|
||
| e.preventDefault(); | ||
| }); | ||
|
|
||
| function updateUI(uuid, type, phone) { | ||
| // Hide login form and show user info | ||
| $login.slideUp(300).attr('aria-hidden','true'); | ||
| $manage.slideDown(300).attr('aria-hidden','false'); | ||
|
|
||
| // NOTE: this handling isn't quite correct, so avoid refactoring without full testing | ||
| var dataTypeDriver = data.type === 'driver'; | ||
| var dataTypeRider = data.type === 'rider'; | ||
|
|
||
| $manage.find('#btnCancelDriveOffer').toggle(dataTypeDriver); | ||
| $manage.find('#btnPauseDriverMatch').toggle(dataTypeDriver); | ||
| $manage.find('#btnCancelMatch').toggle(data.uuid_driver !== undefined && data.score !== 'undefined'); | ||
|
|
||
| $manage.find('#btnCancelRideRequest').toggle(dataTypeRider); | ||
|
|
||
| $manage.find('#btnAcceptDriverMatch').toggle(dataTypeRider && data.uuid_driver !== undefined && data.score !== 'undefined'); | ||
| } | ||
|
|
@@ -110,30 +108,14 @@ function updateUIbyRiderStatus (riderStatus) { | |
|
|
||
| $manage | ||
| .on('click', '#btnCancelRideRequest', cancelRideRequest) | ||
| .on('click', '#btnCancelMatch', cancelMatch) | ||
| .on('click', '#btnCancelDriveOffer', cancelDriveOffer) | ||
| .on('click', '#btnPauseDriverMatch', pauseDriverMatch) | ||
| .on('click', '#btnAcceptDriverMatch', acceptDriverMatch) | ||
| .on('click', '#logout', logout); | ||
|
|
||
|
|
||
| function cancelMatch() { | ||
| if (!window.confirm('This will cancel your ride match. Are you sure you want to proceed?')) { | ||
| return; | ||
| } | ||
|
|
@@ -144,7 +126,7 @@ function cancelDriverMatch() { | |
| Score: data.score, | ||
| RiderPhone: data.phone | ||
| }, | ||
| '/cancel-' + data.type + '-match' | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -225,8 +207,6 @@ function driverExists () { | |
| if (keys[0] == "driver_exists" && info === "") { | ||
| driverLoggedIn = true; | ||
|
|
||
| updateUI(); | ||
|
|
||
| clearDriverInfo (); | ||
|
|
@@ -619,8 +599,6 @@ function riderExists () { | |
| if (keys[0] == "rider_exists" && info === "") { | ||
| riderLoggedIn = true; | ||
|
|
||
| updateUI(); | ||
|
|
||
| clearRiderInfo (); | ||
|
|
@@ -799,20 +777,7 @@ function cancelRideRequest() { | |
| } | ||
|
|
||
| function createAPIurl (params, apiRoute) { | ||
|
Collaborator
There was a problem hiding this comment. I think this change might reflect our different coding styles. The tq lib is fantastic for changing the banners on the site for different partners, but I find when I come back to this code after a break, that it is slower for me to work out what is happening from the tq function name (set()) than reading some code. Usually, I have to find the library code and read that.
Member
Author
There was a problem hiding this comment. Noted, thanks! I just felt it would help simplify things. The library is already included so I thought, why not use it? The documentation is here: https://github.com/richardwestenra/tiny-query-string#set - it's linked to from the file where the library is included. I guess the method name isn't as clear as it could've been - what would you have called it?
Collaborator
There was a problem hiding this comment. @richardwestenra This just feels like a coding style thing, to me, which is absolutely fine. My style, in the lack of a framework (not inherently an issue), is just to use the standard browser API throughout this page. For me, that API is widely documented and doesn’t give contributors anything new to learn. Still, there are at least two sides to that debate. My general thoughts are the code should suit whoever is mostly taking ownership of the page. If that will be you, great, go ahead and make use of any libraries that can help. The name should match the behaviour if possible. I wasn’t sure if there were any side-effects to the lib function, and so issues arise if called twice before a call, etc? |
||
| return tinyQuery.set(params, remoteUrl + apiRoute); | ||
| } | ||
|
|
||
| function accessCarpoolvoteAPI (url, handlerFunction) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,15 @@ | ||
| var queryStringData = tinyQuery.getAll(); | ||
|
|
||
| if (queryStringData.code !== '0') { | ||
| $('#thanks').hide(); | ||
|
|
||
| var responseInfo = ['code', 'info'].filter(function(d) { | ||
| return queryStringData[d]; | ||
| }) | ||
| .map(function(d) { | ||
| return '<p><small>Response ' + d + ': <b>' + queryStringData[d] + '</b></small></p>'; | ||
| }) | ||
| .join(''); | ||
|
|
||
| $('#sorry').show().append(responseInfo); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This merging of the two cancel functions and buttons looks like a very promising refactor. I've probably not got time to check it before the Nov 7 election, and the changes to element ids may stop my automated tests from working. With the limited time and support team numbers, I'd like to keep those tests going if possible. Can we put this change to one side until after Nov 7, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let's update the tests when we merge after Nov 7.