jpasilan / my_ad

My-Ad Project
0 stars 1 forks source link

Implement the Initial Static Pages #1

Closed jpasilan closed 10 years ago

jpasilan commented 10 years ago

Implement the primary static, public-visible pages. To complete this enhancement the following pages (along with their routes) must already be done and properly styled:

The rest of the pages will follow in another issue.

jpasilan commented 10 years ago

Vincent,

Continue on working on this and just focus on the listed pages. Make sure to test and have it look like the same as the mainbeta subfolder. Work on this on the branch: static-pages

jpasilan commented 10 years ago

Vincent,

Upon fixing on merge conflicts, I've seen this:

                            <ul class="list-unstyled">
<<<<<<< HEAD
                                <li><a href="#"><img src="{{ URL::asset('assets/css/images/google_plus.png') }}" /></a></li>
                                <li><a href="#"><img src="{{ URL::asset('assets/css/images/facebook.png') }}" /></a></li>
                                <li><a href="#"><img src="{{ URL::asset('assets/css/images/twitter.png') }}" /></a></li>
=======
                                <a href="#"><img src="{{ URL::asset('assets/css/images/google_plus.png') }}" /></a>
                                <a href="#"><img src="{{ URL::asset('assets/css/images/facebook.png') }}" /></a>
                                <a href="#"><img src="{{ URL::asset('assets/css/images/twitter.png') }}" /></a>
>>>>>>> static-pages
                            </ul>

Although this looks good on the browser but we always have to follow the proper semantic markup. Omitting changes likes these from the merge.

jpasilan commented 10 years ago

I've already done a merge from the master branch for you. Please check and fix any UI issues that you see. And before I forget, add docblock comments to the controller methods.

jpasilan commented 10 years ago

Merged static-pages (8e42cdd) to the master branch.

jpasilan commented 10 years ago

One last thing for this (I guess). Would you agree that it would be better to use affix (http://getbootstrap.com/javascript/#affix) rather than the accordion for the privacy policy and terms pages. With this, you can replace the nav-pills on the left (which I think is already redundant with the same links already present at the footer) with the listing and use it to implement the affix component.

vpanugaling commented 10 years ago

Yes, affix would be much better than the accordion style. Also I've added the scrollspy component with the affix, for highlighting the side navigation.

jpasilan commented 10 years ago

Forgot about the vission and mission page. Please do implement it as well.

jpasilan commented 10 years ago

Also, a question, why is it that you have set the column widths in the header and navbar to col-md-10 instead of col-md-12? There's so much space on the side when the screen resolution gets larger with this setting.

vpanugaling commented 10 years ago

The column widths class in the header and navbar col-md/ms-10 is already there when I got the html mark-up from the beta server. You are right, it should have the maximum grid(12) in every row not only in header and navbar.

I will just update it from col-md-10 to col-md-12.

jpasilan commented 10 years ago

Ok, finish up this one and the vision and mission pages so that we can already close this ticket.

jpasilan commented 10 years ago

Commits up to 16a7e64 already merged to master.

jpasilan commented 10 years ago

Vincent,

Work on the changes that Ritche wanted to see. If I remember it right it was the login form on the navbar and the 3 columns containing the categories, search form, and the banner ads. Put as a comment in this issue anything that I may have forgotten.

jpasilan commented 10 years ago

Merge from master since there were hotfixes made in it.

vpanugaling commented 10 years ago

Also, he wanted the categories icons to have the correct dimensions when viewed on smaller screens, because as for now it creates unequal dimensions.

Another change is for different search form for Vehicles and Real Estate.

jpasilan commented 10 years ago

Implement those that are in your comment. Also checked your latest commit and it appears that the Advertisement image placeholder is wrongly spelled.

jpasilan commented 10 years ago

Vincent,

Did you merge from the master branch before working on e04082f? If so, I have just observed that your changes have caused regressions. See and compare https://github.com/jpasilan/my_ad/blob/28ac5c9ad135cb84c4702b833d88190062e59657/app/views/layouts/navbar.blade.php with your change here https://github.com/jpasilan/my_ad/blob/e04082f8bcb9ed4c1a29fc402bf7c83b448d74c5/app/views/layouts/navbar.blade.php. Look at lines 42 to 50+.

As always and when merging from master and resolving conflicts, the changes in the master branch should take precedence over the specific branches that we're working work. Regressions are hard to detect and it will be difficult, especially for me, to determine what's missing when deploying site updates.

So since a regression has occurred, I'll give you the hard part of comparing all of your changes with that of the master branch and making sure that everything that's missing are added in the static-pages branch.

jpasilan commented 10 years ago

Merged to master. Closing issue.