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

Simple editor template #6817

Closed
wants to merge 26 commits into from

Conversation

cesswairimu
Copy link
Collaborator

@cesswairimu cesswairimu commented Nov 20, 2019

Fixes #6866
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

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

Thanks!

@jywarren
Copy link
Member

jywarren commented Nov 20, 2019 via email

@cesswairimu
Copy link
Collaborator Author

Aha cool... Will do. Thanks

@jywarren
Copy link
Member

Cool!

@cesswairimu
Copy link
Collaborator Author

@jywarren do we want to add another node type for these so that we can have the show page looking like

new_wiki

or any ideas on how to go about this? Thanks

@jywarren
Copy link
Member

jywarren commented Nov 22, 2019 via email

@cesswairimu
Copy link
Collaborator Author

Sounds good...This is great. Thanks

@cesswairimu cesswairimu force-pushed the simple-editor-template branch 2 times, most recently from 858aefa to a38a0a2 Compare November 27, 2019 13:18
@cesswairimu
Copy link
Collaborator Author

cesswairimu commented Nov 27, 2019

seems like my route naming was the prob 🤦‍♀️ ...moved from /post/simple/ to /simple_post

config/routes.rb Outdated
@@ -308,7 +308,7 @@

get 'post' => 'editor#post', :as => :editor_post
post 'post' => 'editor#post', :as => :editor_path
get 'post/simple' => 'editor#simple'
get '/simple_post' => 'editor#simple'
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, what was the problem here? I liked /post/simple -- was it not finding this in the correct precedence order? Would it work if you moved /post/simple above line 309?

Copy link
Collaborator Author

@cesswairimu cesswairimu Nov 27, 2019

Choose a reason for hiding this comment

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

The capybara test was failing on travis with a error undefined route '/post/undefined....` trying to find another fix

Copy link
Collaborator Author

@cesswairimu cesswairimu Nov 27, 2019

Choose a reason for hiding this comment

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

The error is ActionController::RoutingError: ActionController::RoutingError: No route matches [GET] "/post/undefined"

Copy link
Member

Choose a reason for hiding this comment

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

Weird! Let me see your capybara tests... can you mark what line of tests fails?

fill_in("username-login", with: "steff1")
fill_in("password-signup", with: "secretive")
click_on "Log in"
visit(simple_editor_path)
Copy link
Member

Choose a reason for hiding this comment

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

Aha what if you say visit('/post/simple') here, and don't use the Rails path generator?

Copy link
Collaborator Author

@cesswairimu cesswairimu Nov 28, 2019

Choose a reason for hiding this comment

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

Having the path as post/simple is causing the error...It is also visible on my server locally..I didn't catch this earlier 😥 I think its a rails error... 🤔 maybe because we do not have a post resource...it expects this to be like editor/simple because we have editor controller...maybe

@jywarren
Copy link
Member

Getting there!

@cesswairimu cesswairimu force-pushed the simple-editor-template branch 2 times, most recently from c8b5304 to d1a5acc Compare November 28, 2019 15:53
@jywarren
Copy link
Member

jywarren commented Nov 28, 2019 via email

@cesswairimu cesswairimu force-pushed the simple-editor-template branch 3 times, most recently from 3ecdd46 to cac992e Compare November 28, 2019 19:54
@cesswairimu
Copy link
Collaborator Author

Hi @jywarren, I am still getting a hold of how the inline-maps works...I have realized that if the tag place is added..the post appears on top...do we need to add a place tag by default on this template or do we know that the user will know that the they need to add the tag?

Also on the map type do we want to leave it as is..or have it with bluish color as it was on the mockup..Thanks

@jywarren
Copy link
Member

jywarren commented Dec 9, 2019

Ah, sorry I was ambiguous! the map should appear on the top in the simple editor, not the resulting published page -- so, in the editor template, I think you could move the map module to the very top (make it the first module) and use CSS to style it! Does this make sense?

@jywarren
Copy link
Member

jywarren commented Dec 9, 2019

And as to the map color, let's switch it to the layer type we were using on the front page of MapKnitter, it looks so nice!

@jywarren
Copy link
Member

jywarren commented Dec 9, 2019

Ah you already made it the first module! Let's then do a couple things - let's use CSS to move the "Add a Location" button and let's get rid of the title at the top of the page - let's just use CSS (and esp. Bootstrap styles! https://getbootstrap.com/docs/4.3/components/) to get this editor to look a lot more like the mockups, what do you think?

@cesswairimu
Copy link
Collaborator Author

cesswairimu commented Dec 10, 2019

Sounds great...thanks Jeff

@cesswairimu cesswairimu force-pushed the simple-editor-template branch 5 times, most recently from 7a2f2a3 to 2c1a242 Compare December 12, 2019 19:28
@gitpod-io
Copy link

gitpod-io bot commented Dec 21, 2020

@codeclimate
Copy link

codeclimate bot commented Dec 21, 2020

Code Climate has analyzed commit a24f08f and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Style 1

View more on Code Climate.

@stale
Copy link

stale bot commented Dec 21, 2021

Hi 😄, this issue has been automatically marked as stale because it has not had recent activity. Don't worry you can continue to work on this and ask @publiclab/reviewers to add in-progress label 🎉 . Otherwise, it will be closed if no further activity occurs in 10 days -- but you can always re-open it if you like! 💯 Thank you for your contributions! 🙌 🎈.

@stale stale bot added the stale label Dec 21, 2021
@stale stale bot closed this Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple Editor template
4 participants