Skip to content

Issue 461#665

Merged
joshi1983 merged 13 commits intohhaccessibility:masterfrom
neodes-h:issue-461
Aug 7, 2018
Merged

Issue 461#665
joshi1983 merged 13 commits intohhaccessibility:masterfrom
neodes-h:issue-461

Conversation

@neodes-h
Copy link
Copy Markdown
Collaborator

Here are some screenshots.
image
image
image

Comment thread app/routes/location.php Outdated
Route::post('api/set-search-radius', 'LocationSearchController@setSearchRadius');
Route::post('api/add-suggestion', 'SuggestionController@addSuggestion');
Route::get('suggestion-list/{location_id}','SuggestionController@showSuggestionList');
Route::get('suggestion-detail/{suggestion_id}','SuggestionController@showSuggestionDetail');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please run "composer all" and fix the problems that it finds.

Here is what I get when I run it.
image

@section('content')
<div class="suggestion-list">
<div class="text-center">
<h1><a href="/location/management/my-locations">Suggesstions for {{ $name }}</a></h1>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix spelling. "Suggesstions" should be "Suggestions".

<div class="suggestion-list">
<div class="text-center">
<h1><a href="/location/management/my-locations">Suggesstions for {{ $name }}</a></h1>
</div>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When no suggestions are on a specific location, it would be nice to have a message saying that.

image

That would be consistent with how My locations:
image

<div class="col-xs-4">
<a href="/location/rating/{{ $location->id }}">{{ $location->name }}</a>
<div class="col-xs-3">
<a href="/location-rating/{{ $location->id }}">{{ $location->name }}</a>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like some rebasing mistakes were made here with route changes that got merged Tuesday. The new route for rating is "/location/rating/" instead of "/location-rating/" so your changes would break links in the My Locations page.

Test those other links on the "My Locations" list when you're done fixing.

@section('content')
<div class="suggestion-detail">
<div class="text-center">
<h1>{{ $user_name }}'s <a href="/suggestion-list/{{ $suggestion->location_id }}">Suggesstion for {{ $location_name }}</a></h1>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Replace "Suggesstion" with "Suggestion" in suggestion_detail.blade.php.

<div class="list-group">
@foreach($suggestions as $suggestion)
<a href="/suggestion-detail/{{ $suggestion->id }}" class="list-group-item" title="Click to check details">
<span class="username">Generated by: {{ $suggestion->user_name }}</span>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of "Generated", use "Submitted". "Generated" sounds more like it was made by a machine.

@import "_sign_in.scss";
@import "_sign_up.scss";
@import "_social_media_buttons.scss";
@import "_suggestion_list.scss";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like that you followed the way the code was organized here by making files specific to each page.

@@ -0,0 +1,28 @@
@import "_placeholders.scss";

.suggestion-detail{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add spaces immediately before the { to be more consistent with the current scss files.

I marked in red the space from a file that existed before you made any changes:
image

@@ -0,0 +1,57 @@
@extends('layouts.default')
@section('head-content')
<script src="/js/suggestion_detail.js"></script>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Swap the script elements for including suggestion_detail.js with jquery so $ is defined before you run suggestion_detail.js.

$location = DB::table('location')
->where('id','=',$suggestion->location_id)
->get(['name','external_web_url','address','phone_number'])[0];
$name = DB::table('user')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just for your information, you can use the model classes to reference the tables instead of DB::table.

For example, "Location::find($location_id)"

@joshi1983 joshi1983 merged commit 8b40c1a into hhaccessibility:master Aug 7, 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.

2 participants