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

Sidebar: Add toggle icons #6011

Merged
merged 1 commit into from Jul 26, 2019

Conversation

@CleverFool77
Copy link
Member

commented Jul 12, 2019

Fixes #6005 part

Screenshot from 2019-07-12 16-37-59

Screenshot from 2019-07-12 16-38-19

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch from 7db2bda to 6dc0fcc Jul 12, 2019

@CleverFool77 CleverFool77 added this to In progress PRs in UI improvements - Summer Of Code 2019 via automation Jul 12, 2019

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch from 6dc0fcc to dc9cb15 Jul 12, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

I removed the Write a post button, Ask question button and test related to it as it as not included in new design.
Thanks !!!

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch 2 times, most recently from 354010c to 0a31f28 Jul 12, 2019

UI improvements - Summer Of Code 2019 automation moved this from In progress PRs to Done Jul 12, 2019

@CleverFool77 CleverFool77 reopened this Jul 12, 2019

UI improvements - Summer Of Code 2019 automation moved this from Done to In progress PRs Jul 12, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

'This system test for location-modal which is failing seems unrelated.

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

I guess I need to do some refactoring on the way code is written because the screenshot is going well for note but not wiki.

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch from 0a31f28 to b273a0b Jul 12, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

This is the diff in ss of both wiki and notes

WIKI

Screenshot from 2019-07-12 19-23-01

NOTE

Screenshot from 2019-07-12 19-23-15

But here I came across one problem in wiki. I'm getting this 🤔 and I checked it through console. And Right now trying the resolve and think the problem behind this.

Screenshot from 2019-07-12 19-26-11

cc : @gautamig54 @jywarren

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 23, 2019

HI @gautamig54 @jywarren the code part of some portion of toggle icons is written in one cell whereas some other part is written in different cell which is leading to different amount of divs and thus distortion in UI. So Should i just refactor the whole code from all cells for toggle icons to one cell ? 🤔 or Should I just try to find a fix by letting it remain in different cells only.

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Hi @jywarren @gautamig54 @gauravano Any idea that what could be the reason for this ?
The code part of unusual toggle icons on left of main title is not present in while checking through inspect element. Is it because of li mismatch ? 🤔
Screenshot from 2019-07-24 15-47-11

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch from b273a0b to ca3fd58 Jul 24, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Hi , So finally I found the reason of this extra toggle icons. And Now updated the PR.
These are the ss:

Wiki page

Screenshot from 2019-07-24 16-04-26

Note page

Screenshot from 2019-07-24 16-05-39

I guess this PR is ready for review now.

@CleverFool77 CleverFool77 requested a review from jywarren Jul 24, 2019

@CleverFool77 CleverFool77 requested a review from gautamig54 Jul 24, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Hi @jywarren @gautamig54 It would be great if you could review this.
Thanks !!

@jywarren

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2019

This is great. Can you just in list form walk through the meaning of each icon, and are there tooltips for them all? Thank you!

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Hi @jywarren
Yes!! They have tooltip and all the info that was being shown earlier is there in them.
ss attached :
Screenshot from 2019-07-25 01-41-41

Screenshot from 2019-07-25 01-41-54
Screenshot from 2019-07-25 01-41-58

@jywarren

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2019

@CleverFool77 CleverFool77 force-pushed the CleverFool77:toggle-icons branch from ca3fd58 to e23b3a4 Jul 24, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Turned border color lighter to #ccc

Screenshot from 2019-07-25 02-11-09

@plotsbot

This comment has been minimized.

Copy link
Collaborator

commented Jul 24, 2019

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

6011-test_viewing_question_post.png

6011-test_wiki.png

6011-test_tag_page.png

6011-test_blog_page_with_location_modal.png

6011-test_login.png

6011-test_wiki_page_with_inline_grids.png

6011-test_questions.png

6011-test_methods.png

6011-test_tag_by_author_page.png

6011-test_viewing_the_dropdown_menu.png

6011-test_comments.png

6011-test_stats.png

6011-test_tags.png

6011-test_people.png

6011-test_front.png

6011-test_signup.png

6011-test_questions_shadow.png

6011-test_blog.png

6011-test_question_page.png

6011-test_front_page_with_navbar_search_autocomplete.png

6011-test_viewing_the_dashboard.png

Learn about automated screenshots

Generated by 🚫 Danger

<p><a id='print-command-3-col'><i class='fa fa-print'></i> Print in 3-column layout</a></p>
</div>
">
<span class="ff fa fa-copy"></span>

This comment has been minimized.

Copy link
@jywarren

jywarren Jul 24, 2019

Contributor

This is the only one I'm not sure of. Maybe this should be a ... menu instead? I'm just worried the 2-pages icon isn't clear enough. What do you think?

This comment has been minimized.

Copy link
@CleverFool77

CleverFool77 Jul 25, 2019

Author Member

So What does that copy icon in style guide mean ? I thought copy icon looks like pages , so its something similar to print of 2 pages and other stuff menu.

This comment has been minimized.

Copy link
@CleverFool77

CleverFool77 Jul 25, 2019

Author Member

Or should I just remove copy icon from this and just have ... menu ?

This comment has been minimized.

Copy link
@CleverFool77

CleverFool77 Jul 25, 2019

Author Member

Hi @jywarren The style guide doesn't seem to have this menu icon in sidebar ? Instead it's usually in main left column.

Screenshot from 2019-07-25 15-15-57

@jywarren

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

Lets have copy icon as subpages and all rest stuff in bullet menu. Is that right ?

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

Hi @jywarren Just updated this PR.
screenshot attached:

Screenshot from 2019-07-25 23-30-50
Screenshot from 2019-07-25 23-31-03
Screenshot from 2019-07-25 23-31-23

@jywarren

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

@CleverFool77

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

@CleverFool77 CleverFool77 requested a review from jywarren Jul 26, 2019

@jywarren jywarren merged commit 4e5488b into publiclab:master Jul 26, 2019

3 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
danger/danger ⚠️ 1 Warning. Don't worry, everything is fixable.
Details

UI improvements - Summer Of Code 2019 automation moved this from In progress PRs to Done Jul 26, 2019

@jywarren

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

Awesome!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.