-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[WIP]Make wiki/note page headings more minimal, based on the new style guide #6758
[WIP]Make wiki/note page headings more minimal, based on the new style guide #6758
Conversation
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
ee67e9c
to
a466d1c
Compare
Hi this is awesome! We're you able to move the extra items into one of the menus instead? Thanks! |
Hey @jywarren, did you mean move the extra stuff that is removed like views, comments, etc of the wiki and notes to the sidebar button on the top right corner? |
That's right - the round button with the Also note how we put the "edit" button in a circular button on pages like this: https://publiclab.org/notes/warren/11-30-2017/build-a-papercraft-spectrometer-for-your-phone-version-2-0 It'd be great to do the same with wiki pages like this! Do you think you could try implementing that? |
These could fit into this dropdown here: I think the code for this section is here: https://github.com/publiclab/plots2/blob/master/app/views/like/_like.html.erb |
I can definately do that. Okay. Thank you so much for the details. I will implement it soon. |
Thanks, that's awesome! Just ping me if you need any help! |
Hi, just checking if you've gotten stuck on this at all, or if I could help in any way? Thanks! |
Hey, I'm not stuck. I'm just having my exams so I'm a bit busy. But, i will try to do it by this weekend hopefully. Thanks |
a466d1c
to
e82b81d
Compare
e82b81d
to
81e2a92
Compare
Hi, looks good! Can you upload a screenshot or gif of the menu open? Thanks! |
This looks great. Now it looks like a few tests that had been expecting to find "views" listed in the old location are now failing. Could you try adjusting the tests to look in the new location? The problem may be that the popups are constructed in JavaScript so they may not be actually visible to the functional tests (i.e. they don't appear in plain HTML). So we could remove them, or test for something else, or we could move them to system tests, which do run JavaScript.
I guess the simplest seems to me to be just changing them to test for some of the new header text you've reduced things down to, rather than the text that is now hidden in the popover. What do you think? |
Oh you know, actually this kind of looks like it's not failing in the view, but actually somehow we're passing a nil into the tests and they're trying to get |
Okay. I will try to figure it out. |
app/views/like/_like.html.erb
Outdated
<div> | ||
<span> | ||
<% if node.type == 'note' %> | ||
<%= number_with_delimiter(@node.views) %> <%= t('notes.show.views') %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha - here, i think we just need node.views
and not @node.views
! I'm going to try making this change to see if it works!!
Screenshots 📸 (click to expand)6758-test_questions.png6758-test_embeddable_grids.png6758-test_signup.png6758-test_viewing_the_settings_page.png6758-test_tag_by_author_page.png6758-test_wiki_page_with_inline_grids.png6758-test_stats.png6758-test_viewing_the_dashboard.png6758-test_searching_an_item_from_the_homepage.png6758-test_questions_shadow.png6758-test_login_modal.png6758-test_profile_page.png6758-test_comments.png6758-test_tags.png6758-test_signup_modal.png6758-test_wiki.png6758-test_methods.png6758-test_tag_page.png6758-test_blog_page_with_location_modal.png6758-test_tag_wildcard.png6758-test_embeddable_thumbnail_grids.png6758-test_front_page_with_navbar_search_autocomplete.png6758-test_login.png6758-test_viewing_the_dropdown_menu.png6758-test_viewing_question_post.png6758-test_mobile_displays.png6758-test_simple-data-grapher_powertag.png6758-test_front.png6758-test_question_page.png6758-test_tag_contributors_page.png6758-test_blog.png6758-test_people.png6758-test_wiki_revisions.pngLearn about automated screenshots Generated by 🚫 Danger |
Hooray!!! This looks great. The test failure was pretty simple - just removing |
Congrats on merging your first pull request! 🙌🎉⚡️ Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
That was easy! Thank you so much for helping me out. :) |
No problem! Thank you!!
…On Wed, Dec 4, 2019 at 4:57 AM Umang taneja ***@***.***> wrote:
Hooray!!! This looks great. The test failure was pretty simple - just
removing @ from some lines. Thanks a ton!!!
That was easy! Thank you so much for helping me out. :)
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#6758?email_source=notifications&email_token=AAAF6J2X73462RJK67WUKRLQW55IJA5CNFSM4JLI6KC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF4MW6Y#issuecomment-561564539>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF6JZQVZRUL2L6H5264VDQW55IJANCNFSM4JLI6KCQ>
.
|
…e guide (publiclab#6758) * Make wiki/note page headings minimal, based on the style guide * remove @
Fixes #5956
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf 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!