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

post-detail page #93

Merged
merged 33 commits into from Sep 26, 2013
Merged

post-detail page #93

merged 33 commits into from Sep 26, 2013

Conversation

kamaulynder
Copy link
Contributor

@rjmackay pushed the post-detail stuff, need help with css before I can do anything else. Added the latest global.css file from the UI repo and that affected the post-listing too, argghhhh!!

@rjmackay
Copy link
Contributor

I think the best way to handle this might be to do HTML/CSS updates on their own branch. The rest of the time just wire things in and don't worry too much if the CSS isn't perfect on the feature branches.
Either that or we tag the UI repo each time we sync the HTML/CSS and build new UI based on those tags.
I think the last update I did was based on https://github.com/ushahidi/Lamu-UI/commits/ed164de .. so maybe revert your CSS update and base the HTML on UI @ that commit?

Also - when you update the CSS - grab scss/ files from the UI repo and run grunt compass rather than copying the built CSS. This will build the CSS uncompressed (for debugging), rather than compressed like Seth is doing.

@@ -1,9 +1,11 @@
define(['App', 'backbone', 'marionette',
'views/AppLayout', 'views/HomeLayout', 'views/HeaderView', 'views/FooterView', 'views/WorkspacePanelView', 'views/SearchBarView', 'views/MapView',
'views/PostListView', 'views/PostDetailView','collections/PostCollection','collections/TagCollection','collections/FormCollection'],
'views/PostListView',
'views/PostDetailView','collections/PostCollection','collections/TagCollection','collections/FormCollection','views/PostDetailLayout', 'views/RelatedPostsView'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you break this up and try and avoid the really long line? I've been doing something like
a line of layouts, then views, the collections.. etc

little pedantic I know.. but just thinking readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoah, so moving around this as per the suggestion breaks the whole app, not sure I'm getting it right but put all layouts on one line, views on another and collections on one line. Breaks whole UI.

Copy link
Contributor

Choose a reason for hiding this comment

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

You have to match the ordering to the function params below.. as long as ordering matches you should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done and works

@kamaulynder
Copy link
Contributor Author

And I ended up creating more problems to myself logs off 1am commits are not the best

@kamaulynder
Copy link
Contributor Author

@rjmackay - for the css I think it can wait till its updated, running away from revert madness :-).
Did some edits as per your review.

@rjmackay
Copy link
Contributor

Some git madness will revert it:
git rebase -i
then set 3c0ceca to edit.. and use git commit --amend to remove the css from that commit..

or slightly simpler

git diff 3c0ceca..e4080ba -- modules/UshahidiUI/media/css/ | git apply
git add modules/UshahidiUI/media/css
git commit

which basically diff's the commit with css against the previous one (reverse order so you get a reversed diff), filters it to just the css directory.. and then pipes it into git apply to patch it.

@kamaulynder
Copy link
Contributor Author

Done!

On Thu, Aug 22, 2013 at 2:35 AM, Robbie MacKay notifications@github.comwrote:

Some git madness will revert it:
git rebase -i
then set 3c0ceca 3c0ceca to
edit.. and use git commit --amend to remove the css from that commit..

or slightly simpler
git diff 3c0ceca..e4080ba -- modules/UshahidiUI/media/css/ | git apply
git add modules/UshahidiUI/media/css
git commit
which basically diff's the commit with css against the previous one
(reverse order so you get a reversed diff), filters it to just the css
directory.. and then pipes it into git apply to patch it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-23058764
.

Too brief? Here's why! http://emailcharter.org
Regards
Linda Kamau
Coder and Tech Enthusiast!!

@rjmackay
Copy link
Contributor

@kamaulynder Git-fu: +1
Level Up!

@kamaulynder
Copy link
Contributor Author

hahahaha

* Remove wrapping elements from templates (provided by layout)
* Update some HTML from UI repo
* Fix indenting
* Make sure model gets loaded even if its not in the collection
* Don't keep postDetailLayout around after we go somewhere else
* Fix ID of postDetailRegion
* Fix camelCase on postDetailLayout regions
*
* Move post tags mapping code to post model
* Copy PostItemView.serializeData() to PostDetailView
* Replace dummy html with real data
* Only add color style if needed (postDetailView and postItemView)
This is a rough hack for when the page renders before its model is loaded
@rjmackay
Copy link
Contributor

I've merged some UI updates into this branch, synced up the post-detail UI with the latest, fixed some CSS glitches, and wired up a few more fields with real data.

Outstanding things

  • Pull a real location
  • Pull a real author name
  • Use real location data on the map
  • Get real data into related posts .. just any posts until Related Posts #94 is done
  • Get real images .. can't do this till media works in the API.

Conflicts:
	modules/UshahidiUI/media/js/app/controllers/Controller.js
	modules/UshahidiUI/media/js/app/models/PostModel.js
	modules/UshahidiUI/media/js/app/views/PostDetailView.js
* Use deferreds to ensure post, user and form models all load before
  we render post details
* Store form and user models attached to the post
* Fix template logic to show user
* Fix accessor names on PostModel: getTags isPublished()
* Split out RelatedPostItem template
* Update RelatedPostsView to use PostItemView as item view. Use custom
  options to change the template and class name, but same view code.
* Make item container a <ul> and update CSS to remove left margin
@rjmackay
Copy link
Contributor

Go related posts working (with dummy post models. However I think the HTML itself needs some work.. theres nothing standard between the main listing and side lists.. which will be a bit rough for the themers.

@rjmackay
Copy link
Contributor

@kamaulynder I've built a lot on this - it should be pretty much done excluding the post images which are still placeholders. Care to give this a quick review?

@rjmackay
Copy link
Contributor

Also: started writing up a code review checklist: https://wiki.ushahidi.com/display/WIKI/Code+review+checklist
I'll start using this and tweaking as needed..

@kamaulynder
Copy link
Contributor Author

reviewing, thanks.

On Mon, Sep 23, 2013 at 4:08 AM, Robbie MacKay notifications@github.comwrote:

@kamaulynder https://github.com/kamaulynder I've built a lot on this -
it should be pretty much done excluding the post images which are still
placeholders. Care to give this a quick review?


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-24894890
.

Too brief? Here's why! http://emailcharter.org
Regards
Linda Kamau
Coder and Tech Enthusiast!!

@rjmackay
Copy link
Contributor

Ping @middle8media I've made a few changes to the UI during implementing..

Prototype screen shot 2013-09-23 at 5 47 26 pm

App screen shot 2013-09-23 at 5 47 15 pm

  • Remove the category icon beside post title, we don't have a way to establish which is the primary category yet
  • Moved author to the left along with other meta info.. saying "DUDE BRO ADDED A POST" makes sense on a new item, but not several weeks later
  • Location is just lat/lon till I have better info from the API.

No longer needed no we're used deferreds to ensure the model is loaded
@rjmackay
Copy link
Contributor

random thought: should there be a delete button on post details page? I think yes..

@kamaulynder
Copy link
Contributor Author

Yes, I think it should be there. An admin might be going through posts and
find it irrelevant so a quick delete button is necessary
On 23 Sep 2013 10:32, "Robbie MacKay" notifications@github.com wrote:

random thought: should there be a delete button on post details page? I
think yes..


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-24902871
.

@sethburtonhall
Copy link

ok, those changes look good. Except for the paragraph styling and the buttons (they need to be float to line up on the right edge.)

Yes to delete button.

I guess, I just need to jump into the application repo

@rjmackay
Copy link
Contributor

Thanks Seth - not sure what's going on with the paragraph styling.. might be an issue with how I'm formatting the text (I just did a quick new line to br script, until I had time to build in markdown)
The other fixes should be quick..

@rjmackay
Copy link
Contributor

@middle8media am I right that the Post title should also sit in-line with the photo below it?

* Fix column sizes on post title and actions now that category icon is hidden
* Wrap post content in paragraph
@rjmackay
Copy link
Contributor

Added the delete and unpublish buttons.. but it doesn't leave a lot of space for the post title now. Maybe these would be better moved down, just above the post text? or a 2nd 'action bar'?

Screenshot: https://dl.dropboxusercontent.com/u/123897/Screen%20Shot%202013-09-27%20at%2010.33.22%20AM.png

@rjmackay
Copy link
Contributor

I'm going to go ahead an merge this in now anyway. I'll handle further iterations on a new branch. For now it'd like to move on to post search or create

rjmackay added a commit that referenced this pull request Sep 26, 2013
@rjmackay rjmackay merged commit 08dd01c into master Sep 26, 2013
@sethburtonhall
Copy link

I will pull in changes and take a look at this. A 2nd action bar may be our best choice.

nimishmedatwal pushed a commit to nimishmedatwal/platform that referenced this pull request Mar 11, 2024
* badges for filters location
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.

None yet

4 participants