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

Eye icon added infront of followers name #7522

Closed
wants to merge 4 commits into from

Conversation

NitinBhasneria
Copy link
Collaborator

Fixes #6304 (<=== Add issue number here)

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!

@codecov
Copy link

codecov bot commented Feb 17, 2020

Codecov Report

Merging #7522 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7522   +/-   ##
=======================================
  Coverage   81.94%   81.94%           
=======================================
  Files          97       97           
  Lines        5627     5627           
=======================================
  Hits         4611     4611           
  Misses       1016     1016           

@NitinBhasneria
Copy link
Collaborator Author

@cesswairimu Can you review this pr?

@NitinBhasneria NitinBhasneria changed the title Eye icon added infromt of followers name Eye icon added infront of followers name Feb 17, 2020
@VladimirMikulic
Copy link
Contributor

Could you please post a screenshot of your change? Every time you do a UI change, you are required to provide a screenshot show casing the change.

@NitinBhasneria
Copy link
Collaborator Author

As it is a feature the UI change is not shown in screen as for server there are no followers!! @VladimirMikulic

@nstjean
Copy link
Contributor

nstjean commented Feb 17, 2020

So what you will need to do is add some data to your local server so that you can test out the feature. When you open the website at localhost you can log in as user (password:password) and then follow a bunch of topics. Then as admin you can log in to see if it is showing up properly.

@NitinBhasneria
Copy link
Collaborator Author

NitinBhasneria commented Feb 18, 2020

eye1

this is how it will look. @VladimirMikulic @nstjean @cesswairimu

@cesswairimu
Copy link
Collaborator

Hi @NitinBhasneria , this is looking good... I think we can have the >> on the same line as the Notes I think that would look more arranged What do you think?. Also, would like to do the second part of the issue on this PR or should I open another issue for it. Thanks for working on this.

@cesswairimu
Copy link
Collaborator

Another thing, did you remove the other column? please add a screenshot of the whole page. Thanks

@NitinBhasneria
Copy link
Collaborator Author

Hi @NitinBhasneria , this is looking good... I think we can have the >> on the same line as the Notes I think that would look more arranged What do you think?. Also, would like to do the second part of the issue on this PR or should I open another issue for it. Thanks for working on this.

Thanks, @cesswairimu I will arrange that Notes thing in this pr also for the second part please open another issue and close the previous one

@NitinBhasneria
Copy link
Collaborator Author

Another thing, did you remove the other column? please add a screenshot of the whole page. Thanks

Ahh @cesswairimu I am little confused, do we have to remove the follower's column?

@NitinBhasneria
Copy link
Collaborator Author

Notes arranged
Screenshot from 2020-02-19 21-33-42

@cesswairimu
Copy link
Collaborator

Great, the arrangement looks great...No, but we need to just show the followers who are following and are not part of the contributors..how that we have the eye icon showing the contributors who are subscribed..makes sense?

@NitinBhasneria
Copy link
Collaborator Author

NitinBhasneria commented Feb 19, 2020

Yea it does make sense in a way we don't have to check in the follower's list and match if the contributor is also following or not!! @cesswairimu
As, according to the issue, we should show if the contributor if following or not!

@cesswairimu
Copy link
Collaborator

Great...and Jeff suggestion was to rename that column to Following

@NitinBhasneria
Copy link
Collaborator Author

NitinBhasneria commented Feb 19, 2020

So should we make another column named following to show the eye icon?

@cesswairimu
Copy link
Collaborator

Mmmh I think we should rename the one with people who are followers to following and just leave the other one with just the eye icon.....or will it cause confusing?

@NitinBhasneria
Copy link
Collaborator Author

Actually, an eye icon without any heading or column can make it confusing.
So, what I think is making the following column in table one which will have eye icons for the contributors who are following.
and renaming the column named people who are following to following in table two.

@cesswairimu
Copy link
Collaborator

sounds good. Thanks 👍

@NitinBhasneria
Copy link
Collaborator Author

NitinBhasneria commented Feb 19, 2020

So, should I work on this in this pr?
@cesswairimu

@cesswairimu
Copy link
Collaborator

yeah id you are up for it...or we could sseparate it in another issue.totally your choice

@NitinBhasneria
Copy link
Collaborator Author

Yea, we should separate it in another issue.
This will be a less mess then.
Thanks @cesswairimu

@cesswairimu
Copy link
Collaborator

okay cool...then lets add column of the eye icon and I will create another issue of having the followers exclude the contributors. Thanks

@NitinBhasneria
Copy link
Collaborator Author

@cesswairimu This pr work is done and this is how it will look
Screenshot from 2020-02-20 02-13-22
but the followers table has shifted to down instead of right, so this also need to be fixed in another issue.
Thanks
Screenshot from 2020-02-20 02-13-13

@plotsbot
Copy link
Collaborator

1 Warning
⚠️ There was an error with Danger bot’s Junit parsing: No JUnit file was found at output.xml
2 Messages
📖 @NitinBhasneria 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.
📖 #
Screenshots 📸 (click to expand)

7522-test_questions.png

7522-test_embeddable_grids.png

7522-test_signup.png

7522-test_viewing_the_settings_page.png

7522-test_tag_by_author_page.png

7522-test_wiki_page_with_inline_grids.png

7522-test_stats.png

7522-test_viewing_the_dashboard.png

7522-test_searching_an_item_from_the_homepage.png

7522-test_signup_modal_form_validation.png

7522-test_tag_stats.png

7522-test_login_modal_form_validation.png

7522-test_questions_shadow.png

7522-test_login_modal.png

7522-test_profile_page.png

7522-test_comments.png

7522-test_tags.png

7522-test_signup_modal.png

7522-test_wiki.png

7522-test_methods.png

7522-test_tag_page.png

7522-test_blog_page_with_location_modal.png

7522-test_tag_wildcard.png

7522-test_signup_modal_disabled_submit_button_on_empty_username.png

7522-test_embeddable_thumbnail_grids.png

7522-test_front_page_with_navbar_search_autocomplete.png

7522-failures_test_following_the_wiki_author.png

7522-test_spam_moderation_page.png

7522-test_login.png

7522-test_viewing_the_dropdown_menu.png

7522-test_viewing_question_post.png

7522-test_mobile_displays.png

7522-test_simple-data-grapher_powertag.png

7522-test_front.png

7522-test_question_page.png

7522-test_tag_contributors_page.png

7522-test_blog.png

7522-test_people.png

7522-test_wiki_revisions.png

Learn about automated screenshots

Generated by 🚫 Danger

@NitinBhasneria
Copy link
Collaborator Author

@nstjean @cesswairimu I am getting this error in travis. Any idea?
Screenshot from 2020-03-01 12-48-20

@cesswairimu
Copy link
Collaborator

cesswairimu commented Mar 2, 2020

Not sure why it is happening...Is it failing locally?

@NitinBhasneria
Copy link
Collaborator Author

Not sure why it is happening...Is it failing locally?
@cesswairimu
Same travis error in pr #7445

@VladimirMikulic
Copy link
Contributor

@cesswairimu all tests pass locally, but some of them like this one fail in production for some reason.

@cesswairimu
Copy link
Collaborator

I will restart travis one more time...Travis passed on the other PR

@NitinBhasneria
Copy link
Collaborator Author

@cesswairimu You can review it now as all checks have been passed also we can continue this.

@cesswairimu
Copy link
Collaborator

I will push up the alignment of the components so that we can merge it together. Thanks

@NitinBhasneria
Copy link
Collaborator Author

@cesswairimu Please merge this. Thanks

Copy link
Collaborator

@cesswairimu cesswairimu left a comment

Choose a reason for hiding this comment

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

@NitinBhasneria I found a PR that fixed the alignment issue here #7331 you can see the new changes here https://stable.publiclab.org/contributors/soc please rebase and paste the final screenshot here. Thanks

@jywarren jywarren changed the base branch from master to main June 30, 2020 18:06
@cesswairimu
Copy link
Collaborator

@NitinBhasneria could we please get a screenshot for this so that we can merge? also kindly rebase. Thanks

@cesswairimu cesswairimu reopened this Sep 18, 2020
@cesswairimu
Copy link
Collaborator

This PR has been open for a long time and no activity/ reviews requested has not been addressed. We understand you might be busy and engaged on other things. I am closing this for now...please feel free to reopen if you get some time and would like to finish this. Rem to check if its still available before you reopen. Thanks

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.

Reorganize /contributors/___ pages
5 participants