663576 Basic Snippet Body Editor #1

Merged
merged 7 commits into from Jun 16, 2011

Projects

None yet

2 participants

@Osmose
Owner
Osmose commented Jun 15, 2011

Adds a "basic" form

Osmose added some commits Jun 14, 2011
@Osmose Osmose Replace textarea for snippet body with basic/advanced editor c584092
@Osmose Osmose Add ability to convert snippet icons to base64 pngs 629f868
@Osmose Osmose Improve UI, remove auto-icon-embed, and add wiki-links
 - UI rearranged for bigger input boxes
 - User now has to click a button to embed an icon, making it harder
   to lose an icon while making changes
 - Links can be added using a wiki-like syntax:
   [http://link.url|Link text]
62ca26b
@Osmose Osmose Add note to UI about wiki-syntax links 91fe47c
@Osmose Osmose Clean up JS to make more sense f9a6303
@Osmose Osmose Fix bug with init code, add focus to advanced tab on non-basic snippets
 - Init code move inside an anon function so that early returns work
b229806
@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/admin.py
@@ -9,6 +9,10 @@ from django.db.models import get_app, get_apps, get_model, get_models
from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import render_to_response
+from django.utils.encoding import smart_unicode
+from django.utils.safestring import mark_safe
+from django.template.loader import render_to_string
davedash
davedash Jun 15, 2011 Member

I prefer alphabetizing these.

@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/templates/snippetBodyWidget.html
@@ -0,0 +1,23 @@
+<div id="snippet-editor">
+ <ul id="snippet-tabs">
davedash
davedash Jun 15, 2011 Member
<backspace><backspace>
@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/templates/snippetBodyWidget.html
@@ -0,0 +1,23 @@
+<div id="snippet-editor">
+ <ul id="snippet-tabs">
+ <li><a href="#snippet-basic">Basic</a></li>
+ <li><a href="#snippet-advanced">Advanced</a></li>
+ </ul>
+ <div id="snippet-basic" class="panel-container">
+ <div>Snippet Preview:</div>
+ <div id="snippet-preview"></div>
+ <div id="snippet-basic-inputs">
+ <label for="snippet-icon-url">Icon URL (png only):</label>
davedash
davedash Jun 15, 2011 Member

No l10n? cool.

@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/views.py
from django.http import HttpResponseForbidden, HttpResponseNotModified
+from django.utils import simplejson
davedash
davedash Jun 15, 2011 Member

These imports are all over the place.

Do something like this:

import standardlibrary.stuff
import standardlibrary.stuff2

import django.stuff
import django.stuff2

import thirdparty.libs1
import thirdparty.libs2

import local.things1
import local.things2

and no spaces between things in the same group

@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/views.py
from django.conf import settings
from django.core.urlresolvers import reverse
-from django.http import HttpResponseRedirect, HttpResponse
+from django.http import HttpResponseRedirect, HttpResponse, Http404
from django.http import HttpResponseForbidden, HttpResponseNotModified
davedash
davedash Jun 15, 2011 Member

your importing twice from django.http, do it in one line, and use parenthesis if necessary)

or do

from django import http

and then in your code do http.HttpResponse

@davedash davedash commented on the diff Jun 15, 2011
apps/homesnippets/views.py
@@ -79,3 +85,17 @@ def view_snippets(request, **kwargs):
resp['X-FRAME-OPTIONS'] = None
return resp
+
davedash
davedash Jun 15, 2011 Member

two spaces before a function definition

run this code through check.py

@davedash davedash commented on an outdated diff Jun 15, 2011
apps/homesnippets/views.py
@@ -79,3 +85,17 @@ def view_snippets(request, **kwargs):
resp['X-FRAME-OPTIONS'] = None
return resp
+
+@staff_member_required
+def base64_encode(request, **kwargs):
+ """Encode a remote image to base64, and output as JSON"""
+
+ url = kwargs['url']
+ try:
+ img_file = urlopen(url)
+ base64_str = base64.encodestring(img_file.read())
+ except (URLError, ValueError):
+ raise Http404
+
+ return HttpResponse(simplejson.dumps({'img': base64_str}),
davedash
davedash Jun 15, 2011 Member

why simplejson and not json from stdlib?

@davedash davedash commented on an outdated diff Jun 15, 2011
site_media/snippetBodyWidget.js
@@ -0,0 +1,91 @@
+jQuery(function($) {
+ var icon_url_input = $("#snippet-icon-url");
davedash
davedash Jun 15, 2011 Member

I think mozweb convention is single quotes when possible.

Member

Without actually running it, this looks fine to me. Since lorchard has context it'd be nice to have him weigh in. The minor nits are mostly covered in Bootcamp, but everything seems sound.

@Osmose Osmose merged commit 7ba7981 into mozilla:master Jun 16, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment