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

Number of views and comments on small screens #3795

Merged
merged 2 commits into from
Oct 27, 2018

Conversation

tsparksh
Copy link
Member

Fixes #2397

  • 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

@tsparksh
Copy link
Member Author

deepin-screen-recorder_select area_20181026151239
@avsingh999, @Souravirus, please check

@plotsbot
Copy link
Collaborator

plotsbot commented Oct 26, 2018

2 Messages
📖 @thesparks 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

@avsingh999
Copy link
Member

@thesparks great work thanks for contribute, 🎊 😃

@avsingh999
Copy link
Member

avsingh999 commented Oct 26, 2018

@thesparks I think it should be visible, without click on small screen(ex. mobile).
thanks :)

@avsingh999
Copy link
Member

@jywarren @Souravirus @SidharthBansal what do you think about this?
thanks :)

@tsparksh
Copy link
Member Author

@thesparks I think it should be visible, without click on small screen(ex. mobile).
thanks :)

@avsingh999, nooooo, I spent so much time on it((((
Without '...' button I could do it in a couple of minutes 😞
image

@avsingh999
Copy link
Member

@thesparks Sorry, I didn't see that comment.
thanks :)

@avsingh999
Copy link
Member

avsingh999 commented Oct 26, 2018

@thesparks your work is great ,we realy love it, keep contributing 😃

@avsingh999
Copy link
Member

@thesparks one more thing please check this is on moblie view in your browser, it's working fine or not
thanks :)

@tsparksh
Copy link
Member Author

@avsingh999, 👌
deepin-screen-recorder_select area_20181026205713

@avsingh999
Copy link
Member

@jywarren @Souravirus @SidharthBansal It looks good to me,
I think ready to merge

@Souravirus
Copy link
Member

@thesparks I just had one problem with this implementation. The thing is that after you press the '...' button, the screen becomes kind of shaky. So, can you represent the information in two lines? It would be quite good from the user point of view. Thanks for your hard work !!

@jywarren
Copy link
Member

Hi, this looks super! DO you think you could add some padding and a light grey color around the ...? Kind of like in the screenshot of commenting above?

image

That way it's a little more visible! Thank you!!!

@tsparksh
Copy link
Member Author

@Souravirus, please check it yourself, i cant run plots2 (troubles with phantomjs when yarn install).
I think I forgot to include some css in my poc.html (and I already deleted it).
This block must be adaptive in real application

@tsparksh
Copy link
Member Author

@jywarren, Done.
image

@tsparksh
Copy link
Member Author

@Souravirus, this trouble only at GIF, in real app -
image

@Souravirus
Copy link
Member

@thesparks then that's great. Let @jywarren see this and after that, it is good to merge. Thanks!!

@jywarren
Copy link
Member

Awesome!!!

@jywarren jywarren merged commit f423719 into publiclab:master Oct 27, 2018
@ghost ghost removed the ready label Oct 27, 2018
SrinandanPai pushed a commit to SrinandanPai/plots2 that referenced this pull request May 5, 2019
* Update show.html.erb

* collapse-btn style
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.

5 participants