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

Rich editor added , Fixes #904 #1286

Merged
merged 8 commits into from
Mar 13, 2017
Merged

Rich editor added , Fixes #904 #1286

merged 8 commits into from
Mar 13, 2017

Conversation

500swapnil
Copy link
Collaborator

@500swapnil 500swapnil commented Feb 22, 2017

Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!

  • all tests pass -- rake test:all
  • code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
  • pull request is descriptively named with #number reference back to original issue

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays. Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.

Thanks!

@PublicLabBot
Copy link

PublicLabBot commented Feb 22, 2017

3 Messages
📖 @500swapnil Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution!
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the format: Fixes #123.
📖 You have added multiple commits. It’s helpful to squash them if the individual changes are small.

Generated by 🚫 Danger

@500swapnil 500swapnil changed the title Rich editor added , fixes #904 Rich editor added , Fixes #904 Feb 22, 2017
@500swapnil
Copy link
Collaborator Author

@jywarren Let me know how to proceed. Thanks!

@jywarren
Copy link
Member

Hi, have you tried it out manually? Can you post a screenshot? Thanks!

@500swapnil
Copy link
Collaborator Author

screenshot from 2017-02-24 15-05-03
@jywarren Here. Should I remove the 1-2-3 tabs on the side?

@jywarren
Copy link
Member

Hi, actually it looks like you've combined both edit forms -- at about this line, the code for the new one ends and the old one begins:

https://github.com/publiclab/plots2/pull/1286/files#diff-c7e5aa5cf099e96a46ffa9031e9b5a6fR185

You can actually replace the old form code with the new code instead of having both -- see how the /app/views/editor/rich.html.erb contains only the new form code? I think you might start with that template instead of trying to merge them, make sense?

Thank you!

@500swapnil
Copy link
Collaborator Author

@jywarren I gave it the base form of rich editor. What else should we integrate into this page?

@jywarren
Copy link
Member

jywarren commented Mar 1, 2017

I think actually we should try to simplify the page considerably to better match the mockup in #904 -- note how it's just a lot simpler, but has very specific text in the prompts? Want to give that a try?

Thanks!

@500swapnil
Copy link
Collaborator Author

500swapnil commented Mar 3, 2017

screenshot from 2017-03-04 03-11-14
@jywarren I have changed it a little. Should I add the button etc. according to the mockup -- #904 ?

@jywarren
Copy link
Member

jywarren commented Mar 3, 2017

Yes, that'd be great -- give it a try! I think you can just cut & paste the button from its usual place into a new element just below the form.

@500swapnil
Copy link
Collaborator Author

@jywarren I added the Publish and Preview buttons in the last element. This is what the lower half looks like
screenshot from 2017-03-04 03-24-37

@jywarren
Copy link
Member

jywarren commented Mar 4, 2017

Hi, ok, a few more changes, I think --

  • I think we can remove the "Preview" button
  • could you add a "Help" link as shown in the mockup in New form design for asking questions #904, which leads to https://publiclab.org/wiki/questions ?
  • let's remove the numbers on the left
  • can you remove the entire grey bar along the bottom, since we now have a new "Publish" button?
  • can you remove the "Help" columns along the left side as well?

@500swapnil
Copy link
Collaborator Author

500swapnil commented Mar 5, 2017

@jywarren Looks good now?
screenshot from 2017-03-05 06-10-25

@jywarren
Copy link
Member

jywarren commented Mar 6, 2017

I think it's getting very close -- @steviepubliclab, what do you think of this latest mockup vs. the one we put together in #904? We could merge this code and try it out, and do further refinement in a follow-up change, or if there are immediate things you can suggest we can do another few tweaks here.

I don't think this is a stopping concern, but I'd like to see the <hr />s removed -- the horizontal rules. And also I wonder what this looks like in a smartphone view -- with, say, a 500px wide viewport?

Thanks for all your tremendous work on this, @500swapnil -- this kind of design tweaking is a lot of back and forth, but it's really worthwhile when we get it right -- and I appreciate your dedication. We're almost there!

@jywarren
Copy link
Member

jywarren commented Mar 6, 2017

One note -- I guess I'd like the text also to be exactly as we'd posted in the mockup, if possible.

screen shot 2016-10-18 at 12 44 10 pm

Note also the paragraph and bulleted list just above the tagging area, in grey.

@500swapnil
Copy link
Collaborator Author

screenshot from 2017-03-07 01-11-48
@jywarren I think this will work!

@500swapnil
Copy link
Collaborator Author

@jywarren
screenshot from 2017-03-07 01-19-58
Ok I have removed the horizontal rules also. This should be fine!

@500swapnil
Copy link
Collaborator Author

@jywarren Do we need a backlink for Ask button?

@500swapnil
Copy link
Collaborator Author

@jywarren Anything else we need to do?

@ryzokuken
Copy link
Member

@jywarren Please look into this PR, it looks close to being finished. Thanks.

@jywarren
Copy link
Member

Indeed -- let's merge this and give it a try; we can solicit some input from people who try it out and any additional work we might decide on could be done in a followup issue. Thanks!

@jywarren jywarren merged commit 874c9d9 into publiclab:master Mar 13, 2017
@jywarren
Copy link
Member

Thank you, @500swapnil !! I'll hope to publish this today and we can give it a try. If you're looking for another issue, please check out our help-wanted listing -- very grateful for your hard work on this one.

@500swapnil 500swapnil deleted the add-richEditor branch March 14, 2017 19:48
vaibhavgeek pushed a commit to vaibhavgeek/plots2 that referenced this pull request Mar 23, 2017
* Rich editor added

* Starting from rich.html

* Simplified Interface

* publish and preview buttons added

* Bottom bar removed

* Text changed

* hr removed

* Title updated
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