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

Add forward stat page #3812

Merged
merged 7 commits into from
Nov 6, 2018
Merged

Conversation

kevinzluo
Copy link
Collaborator

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

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

  • 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
  • PR is descriptively titled
  • 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

Hello Everyone.
I have added buttons to move forward in the stats page

Thank you,
Kevin Luo

screenshot from 2018-10-27 21-40-31

@plotsbot
Copy link
Collaborator

plotsbot commented Oct 28, 2018

2 Messages
📖 @kevinzluo 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.
📖 It looks like you haven’t marked all the checkboxes. Help us review and accept your suggested changes by going through the steps one by one. If it is still a ‘Work in progresss’, please include ‘[WIP]’ in the title.

Generated by 🚫 Danger

@kevinzluo
Copy link
Collaborator Author

@Souravirus
Sorry to bother, but could you please check this and then confirm on GCI?

Thank you,
Kevin Luo

@Souravirus
Copy link
Member

Nice work @kevinzluo, I was actually traveling yesterday and currently reaching home, that's why didn't see this. I will surely do the review and approve it on gci. Till then can you correct the codeclimate issues. Thank you for your hardwork !!

@@ -17,9 +17,13 @@

<p><%= raw t('notes.stats.looking_back', :time => @time.to_formatted_s(:long)) %></p>
<p>
<%= raw t('notes.stats.try_time_further_back', :url1 => "?time="+(@time-1.year).to_formatted_s(:long),
<%= raw t('notes.stats.try_time_back', :url1 => "?time="+(@time-1.year).to_formatted_s(:long),
Copy link
Member

Choose a reason for hiding this comment

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

@kevinzluo, there is no need to correct this variable here. If there is some issue with this variable, please change it in another PR as we follow one type of change in one PR.

@kevinzluo
Copy link
Collaborator Author

@Souravirus
Thank you for instructing me .
I believe I have correctly done the changes now.

Kevin Luo

@Souravirus
Copy link
Member

Ok nice!! Lets @jywarren See these changes and this would be nice to merge.

@@ -15,6 +15,12 @@ en:
<a href='%{url2}'>one month</a>, or
<a href='%{url3}'>one week</a>
further back.
try_time_further_forward: |
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding these to config/locales/en.yml as we are seeking to consolidate these -- see #3515

Otherwise this is great! Can you upload a screenshot? 📸 Thanks!!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe I added a screenshot to my original pull request, but I will attach another one here.
screenshot from 2018-10-27 21-40-31

Would you like me to move the translations in another PR or commit to this one? Sorry, I am somewhat new to GitHub :( .

Thank you,
Kevin Luo

Copy link
Member

Choose a reason for hiding this comment

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

You can work on it here @kevinzluo

@kevinzluo
Copy link
Collaborator Author

Ok I am sorry but I believe I just messed up my branch.
I will try to fix it.

I am so sorry.

@Souravirus
Copy link
Member

I guess you have merged your branch with the new changes. Although, it won't affect anything, but try to use rebase. It would not show the commits made by others.

@kevinzluo
Copy link
Collaborator Author

Yep. I think I accidentally merged the master into the development branch, and then rebased it, causing all the commits to be out of order.
I believe I have fixed it now but I had to use a reset, which I hear is not very good.

Sorry again,
Kevin Luo

@Souravirus
Copy link
Member

ok nice work @kevinzluo, @jywarren please see this. I think this is complete.

@jywarren
Copy link
Member

jywarren commented Nov 2, 2018

This is beautifully implemented. Thank you so much! Were the translations in the previous location deleted, or are they duplicated? If duplicated, would you mind deleting the previous ones? thank you!!!

@jywarren
Copy link
Member

jywarren commented Nov 6, 2018

Thank you!!!

@jywarren jywarren merged commit 4a69cdd into publiclab:master Nov 6, 2018
@ghost ghost removed the ready label Nov 6, 2018
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* Added options to try time forward on stats page

* changed spacing

* Fixed typo in stats and finished edits.

* Reverting changes

* Added try_time_further_forward

* Hopefully correcting mistakes and moving translations

* Deleting translations that were moved.
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.

4 participants