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.

Sign up for GitHub

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

Jump to bottom

Individual tagpage: Fix absolute wiki image position and add followers to topic #5903

Merged
merged 2 commits into from Jun 21, 2019
Merged

Individual tagpage: Fix absolute wiki image position and add followers to topic #5903

merged 2 commits into from Jun 21, 2019

Conversation

CleverFool77
Copy link
Member

@CleverFool77 CleverFool77 commented Jun 14, 2019 •
edited

Fixes #5890 part

topic with wiki page and image

Screenshot from 2019-06-14 17-32-42

topic with wiki page but no image

Screenshot from 2019-06-14 17-32-54

topic with no wiki page

Screenshot from 2019-06-14 17-32-25

  • 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

@CleverFool77 CleverFool77 changed the title Individual tagpage: Fix abosolute wiki image position and add followers to topic Individual tagpage: Fix absolute wiki image position and add followers to topic Jun 14, 2019
@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 14, 2019 •
edited

Hi @gautamig54 For now, I've commented this part. As you require some of the data from this in next part of the Issue for the main card for topic. L-14 to L-18
<!-- <div class="col-lg-6"> <h1><%= @wiki.latest.title%></h1> <p><%= @wiki.latest.render_body.html_safe.match(/<p>(.+)<\/p>/).to_a.try(:first).to_s.html_safe %></p> <p><%= link_to("Learn more on the wiki page>>", "/wiki/#{params[:id]}") if @wiki.latest.render_body.html_safe.match(/<p>(.+)<\/p>/).to_a.length > 1 %></p> </div> -->

@CleverFool77 CleverFool77 added design issue requires more design work and discussion (i.e. mockups and sketches) outreachy review-me summer-of-code labels Jun 14, 2019
@CleverFool77 CleverFool77 added this to In progress PRs in UI improvements - Summer Of Code 2019 via automation Jun 14, 2019
@plotsbot
Copy link
Collaborator

plotsbot commented Jun 14, 2019 •
edited

1 Warning
⚠️ There was an error with Danger bot’s Junit parsing: No JUnit file was found at output.xml
3 Messages
📖 @CleverFool77 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! And don’t be discouraged if you see errors – we’re here to help.
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the body of your PR, in the format: Fixes #123.
📖 #
Screenshots 📸 (click to expand)

Learn about automated screenshots

Generated by 🚫 Danger

@jywarren
Copy link
Member

Hiii! This looks really good. Notice in the style guide that images that aren't super wide get "letterboxed" like this:

image

Can you show a few examples at different widths? Note that this full-width feature is intended to touch the left and right and top edges.

If it's helpful, it's loosely based on this Froala Design Block:

image

("contents-34", source code here: https://github.com/froala/design-blocks/blob/dev/src/html/contents/contents-34.html)

You can use this "builder" to fetch the code for it; it's in Bootstrap 4: https://www.froala.com/design-blocks/webpage-builder (also to test at different page widths)

Also, I just opened a PR to try generating mobile (iPhone 6) sized screenshots for all the same page: #5905 It's not working but should help us keep an eye on how designs work in mobile too.

@jywarren
Copy link
Member

And -- great work! This is really exciting!

@jywarren
Copy link
Member

And - perhaps This topic has no wiki page. _Click here_ to add one. could work? And if it has no image, I think we can just show no message for now.

Finally - sorry, i know i'm asking a lot of different things! -- could we make the grey color lighter? Maybe #eee ?

THANK YOU!!!! This is awesome!!!! 🚀

@CleverFool77
Copy link
Member Author

And - perhaps This topic has no wiki page. _Click here_ to add one. could work? And if it has no image, I think we can just show no message for now.

Finally - sorry, i know i'm asking a lot of different things! -- could we make the grey color lighter? Maybe #eee ?

THANK YOU!!!! This is awesome!!!!

Hi @jywarren regarding This topic has no wiki page. _Click here_ to add one. we do have button below For adding new wiki. And in the next parts, we'll be changing the UI eventually. That's why I let the button remain below same as of now.

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 14, 2019 •
edited

Hiii! This looks really good. Notice in the style guide that images that aren't super wide get "letterboxed" like this:

image

Can you show a few examples at different widths? Note that this full-width feature is intended to touch the left and right and top edges.

If it's helpful, it's loosely based on this Froala Design Block:

image

("contents-34", source code here: https://github.com/froala/design-blocks/blob/dev/src/html/contents/contents-34.html)

You can use this "builder" to fetch the code for it; it's in Bootstrap 4: https://www.froala.com/design-blocks/webpage-builder (also to test at different page widths)

Also, I just opened a PR to try generating mobile (iPhone 6) sized screenshots for all the same page: #5905 It's not working but should help us keep an eye on how designs work in mobile too.

So If image isn't of full width as needed, then it have to get letterboxed ?

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 14, 2019 •
edited

Hi @jywarren I just pushed the latest changes.
It include -

silver to #eee

Screenshot from 2019-06-14 20-45-22

image getting letterboxed if not of proper size

Screenshot from 2019-06-14 20-55-29

Screenshot from 2019-06-14 20-56-51

image with propoer size taking ful span ?

Screenshot from 2019-06-14 20-48-16

@jywarren
Copy link
Member

jywarren commented Jun 14, 2019 via email

@CleverFool77
Copy link
Member Author

Looking good! Yes, any of these comments could be incorporated in follow-up PRs, no problem! For the mobile view, did you want to work on that a little more before we wrap up this PR and move to the next one? Thanks!
…
On Fri, Jun 14, 2019 at 11:28 AM Lekhika Dugtal @.***> wrote: Hi @jywarren https://github.com/jywarren I just pushed the latest changes. It include - silver to #eee [image: Screenshot from 2019-06-14 20-45-22] https://user-images.githubusercontent.com/26685258/59519869-73ec5080-8ee6-11e9-93ab-d03f23586a07.png testing with mobile screen [image: Screenshot from 2019-06-14 20-43-20] https://user-images.githubusercontent.com/26685258/59519894-7a7ac800-8ee6-11e9-9fa3-14d34ed70db5.png image getting letterboxed if not of proper size [image: Screenshot from 2019-06-14 20-55-29] https://user-images.githubusercontent.com/26685258/59520046-c3328100-8ee6-11e9-925d-2ed537ad3329.png [image: Screenshot from 2019-06-14 20-56-51] https://user-images.githubusercontent.com/26685258/59520133-f543e300-8ee6-11e9-9a90-1f3ffd659736.png image with propoer size taking ful span ? [image: Screenshot from 2019-06-14 20-48-16] https://user-images.githubusercontent.com/26685258/59519998-adbd5700-8ee6-11e9-943a-6c12052f19f5.png — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5903?email_source=notifications&email_token=AAAF6J6IFQFI3LJ5WYYDL43P2O2HFA5CNFSM4HYHHNYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXEAYY#issuecomment-502153315>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J333J4H74AEQGPXIGTP2O2HFANCNFSM4HYHHNYA .

Yuss. I'm working on that.

@jywarren
Copy link
Member

jywarren commented Jun 14, 2019 via email

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 14, 2019 •
edited

Hi @jywarren, Just pushed changes with mobile screen. It's fix of not letting image to out of div with hidden. It looks like temp fix. I'm exploring for better one though.

Screenshot from 2019-06-14 21-20-47

While working on this, I found that this button is longer than the screen of mobile width. But As the UI will eventually chnage. later it will be removed.
Screenshot from 2019-06-14 21-22-23

@jywarren
Copy link
Member

jywarren commented Jun 14, 2019 via email

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 14, 2019 •
edited

Hi @jywarren. I just pushed another set of changes.
So earlier I was doing overflow: hidden which might not be that good as it hides whatever going out of div. Now I tried another method which looks better than previous one.
In this, I have set the maximum width of image to 100%. So when we have mobile view. The span of image is taking full width and not going out of screen.
Though the image might try fit itself inside that width even if it's not of proper size.
How does this sound ?
cc: @gautamig54

@jywarren
Copy link
Member

Can you share some screenshots at different sizes? Thank you!!!!

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 15, 2019 •
edited

Hi @jywarren
I just tried new image.
It looks pretty weird in mobile screen though.
Should I add mobile screens css by using @media ?

Screenshot from 2019-06-15 13-13-23
Screenshot from 2019-06-15 13-13-31

@CleverFool77
Copy link
Member Author

Hi @jywarren Just pushed the latest changes with margin fixes and other changes
It looks similar to style guide now.

image

ezgif-4-5894f59a0de8

Thanks !!

@jywarren
Copy link
Member

jywarren commented Jun 19, 2019 via email

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 20, 2019 •
edited

Hi @jywarren Pushed the latest changes.
Thanks. Now its ready.
screenshot attached.
Screenshot from 2019-06-20 10-34-38

@jywarren
Copy link
Member

Looks perfect. Hmm, why isn't it passing tests?

UI improvements - Summer Of Code 2019 automation moved this from In progress PRs to Done Jun 21, 2019
@CleverFool77 CleverFool77 reopened this Jun 21, 2019
UI improvements - Summer Of Code 2019 automation moved this from Done to In progress PRs Jun 21, 2019
UI improvements - Summer Of Code 2019 automation moved this from In progress PRs to Done Jun 21, 2019
@CleverFool77 CleverFool77 reopened this Jun 21, 2019
UI improvements - Summer Of Code 2019 automation moved this from Done to In progress PRs Jun 21, 2019
@CleverFool77
Copy link
Member Author 

 test_should_have_a_active_asked_and_an_inactive_answered_tab_for_question#TagControllerTest (94.36s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:229:in `block (2 levels) in show_for_author'
            app/controllers/tag_controller.rb:228:in `show_for_author'
            test/functional/tag_controller_test.rb:574:in `block in <class:TagControllerTest>'
ERROR["test_should_have_active_question_tab_for_question", #<Minitest::Reporters::Suite:0x0000556c6e4b7e88 @name="TagControllerTest">, 94.58034166099998]
 test_should_have_active_question_tab_for_question#TagControllerTest (94.58s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:448:in `block in <class:TagControllerTest>'
ERROR["test_should_have_active_question_tab_for_question_for_show_for_author", #<Minitest::Reporters::Suite:0x0000556c6e27ad78 @name="TagControllerTest">, 94.65801051399995]
 test_should_have_active_question_tab_for_question_for_show_for_author#TagControllerTest (94.66s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:229:in `block (2 levels) in show_for_author'
            app/controllers/tag_controller.rb:228:in `show_for_author'
            test/functional/tag_controller_test.rb:564:in `block in <class:TagControllerTest>'
ERROR["test_should_list_answered_questions", #<Minitest::Reporters::Suite:0x0000556c6e12bfa8 @name="TagControllerTest">, 94.7358499]
 test_should_list_answered_questions#TagControllerTest (94.74s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:229:in `block (2 levels) in show_for_author'
            app/controllers/tag_controller.rb:228:in `show_for_author'
            test/functional/tag_controller_test.rb:584:in `block in <class:TagControllerTest>'
ERROR["test_should_list_only_question_in_question_view", #<Minitest::Reporters::Suite:0x0000556c6dbc60e0 @name="TagControllerTest">, 94.93149372399995]
 test_should_list_only_question_in_question_view#TagControllerTest (94.93s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:415:in `block in <class:TagControllerTest>'
ERROR["test_should_take_node_type_as_question_if_tag_is_a_question_tag", #<Minitest::Reporters::Suite:0x0000556c6b85f778 @name="TagControllerTest">, 95.31787420100005]
 test_should_take_node_type_as_question_if_tag_is_a_question_tag#TagControllerTest (95.32s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:399:in `block in <class:TagControllerTest>'
ERROR["test_wildcard_does_not_show_note_for_show_for_author", #<Minitest::Reporters::Suite:0x0000556c69397468 @name="TagControllerTest">, 97.48750839300004]
 test_wildcard_does_not_show_note_for_show_for_author#TagControllerTest (97.49s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:229:in `block (2 levels) in show_for_author'
            app/controllers/tag_controller.rb:228:in `show_for_author'
            test/functional/tag_controller_test.rb:613:in `block in <class:TagControllerTest>'
ERROR["test_wildcard_tag_should_have_a_active_asked_and_an_inactive_answered_tab_for_question", #<Minitest::Reporters::Suite:0x0000556c6734ee18 @name="TagControllerTest">, 97.64335392000004]
 test_wildcard_tag_should_have_a_active_asked_and_an_inactive_answered_tab_for_question#TagControllerTest (97.64s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:243:in `block in <class:TagControllerTest>'
ERROR["test_wildcard_tag_should_list_answered_questions", #<Minitest::Reporters::Suite:0x0000556c66e4ff98 @name="TagControllerTest">, 97.71986454500001]
 test_wildcard_tag_should_list_answered_questions#TagControllerTest (97.72s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:237:in `block in <class:TagControllerTest>'
ERROR["test_wildcard_tag_show", #<Minitest::Reporters::Suite:0x0000556c65795dc0 @name="TagControllerTest">, 97.79505745800003]
 test_wildcard_tag_show#TagControllerTest (97.80s)
ActionView::Template::Error:         ActionView::Template::Error: undefined method `each_with_index' for nil:NilClass
            app/views/users/_answered.html.erb:6:in `_app_views_users__answered_html_erb___2594410514655309534_46962100259300'
            app/views/tag/show/_tab_content.html.erb:33:in `_app_views_tag_show__tab_content_html_erb__1393460966819821938_46962066244360'
            app/views/tag/show.html.erb:45:in `_app_views_tag_show_html_erb__2257457707647204803_46962066452360'
            app/controllers/tag_controller.rb:168:in `block (2 levels) in show'
            app/controllers/tag_controller.rb:167:in `show'
            test/functional/tag_controller_test.rb:218:in `block in <class:TagControllerTest>'
 FAIL["test_should_list_notes_and_questions_in_user_profile", #<Minitest::Reporters::Suite:0x0000556c6da20fb0 @name="UsersControllerTest">, 102.602455486]
 test_should_list_notes_and_questions_in_user_profile#UsersControllerTest (102.60s)
        Expected nil to not be nil.
        test/functional/users_controller_test.rb:155:in `block in <class:UsersControllerTest>'

Hi @jywarren The test are related to tag but not at all related to changes done. Rather these changes will be soon done in next part of Individual tag planning Issue. Right now, I've only added image and followers. So the test regarding tabs, active tabs, question views, authored views, answered tabs shouldn't fail.

@CleverFool77
Copy link
Member Author

HI @jywarren I tested tag_controller locally
using rails test test/functional/tag_controller_test.rb

All the tests which seems to be failing at travis ci passed locally.
I don't seem to understand this, The very same tests which are passing locally is failing on travis ci.

gg

@jywarren
Copy link
Member

jywarren commented Jun 21, 2019 via email

@CleverFool77
Copy link
Member Author

CleverFool77 commented Jun 21, 2019 •
edited

Hi @jywarren
I just rebased and pushed. And all the checks passed 🎉 🎉
Thanks !!

@CleverFool77
Copy link
Member Author

Hi @jywarren I wanted to try writing functional tests, So Can I know from where I should start with or which pages doesn't have much test written on them ?

@jywarren
Copy link
Member

Hi! Due to #5931, our test coverage reporting is a little fragmented. But you can check https://coveralls.io/github/publiclab/plots2 to dig into it, and perhaps we can enable CodeCov for a better look; that has great code analysis tools like this: https://codecov.io/gh/publiclab/image-sequencer

@jywarren jywarren merged commit f4ab778 into publiclab:master Jun 21, 2019
UI improvements - Summer Of Code 2019 automation moved this from In progress PRs to Done Jun 21, 2019
@jywarren
Copy link
Member

Thanks!!!

@CleverFool77 CleverFool77 deleted the image-people branch June 28, 2019 17:42
sagarpreet-chadha pushed a commit that referenced this pull request Jun 29, 2019
@CleverFool77 @sagarpreet-chadha
Individual tagpage: Fix absolute wiki image position and add follower…
768c202 
…s to topic (#5903)

* Fix position of image and add followers count

* Fix color of text
enviro3 pushed a commit to enviro3/plots2 that referenced this pull request Aug 15, 2019
@CleverFool77 @enviro3
Individual tagpage: Fix absolute wiki image position and add follower…
d480984 
…s to topic (publiclab#5903)

* Fix position of image and add followers count

* Fix color of text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gautamig54 gautamig54 Awaiting requested review from gautamig54

@jywarren jywarren Awaiting requested review from jywarren

Assignees
No one assigned
Labels
design issue requires more design work and discussion (i.e. mockups and sketches) outreachy ready summer-of-code
Projects
No open projects
UI improvements - Summer Of Code 2019
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI Improvements Planning Issue : Individual tags page
4 participants
@CleverFool77 @plotsbot @jywarren @gautamig54