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

Hide publish bar when typing on mobile devices #583

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Shulammite-Aso
Copy link
Collaborator

Fixes #435

pulled this out from here #464 so it gets a little easier to review. The original PR had two changes in it

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 grunt jasmine
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • PR body includes fixes #0000-style reference to original issue #
  • 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

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.

Thanks!

@gitpod-io
Copy link

gitpod-io bot commented Jul 31, 2020

Copy link
Contributor

@sagarpreet-chadha sagarpreet-chadha left a comment

Choose a reason for hiding this comment

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

Looks like we have extra things added in /dist file, other than that PR is good to me merged 💯

@Shulammite-Aso
Copy link
Collaborator Author

Looks like we have extra things added in /dist file, other than that PR is good to me merged

Oh! do i have to do anything about this @sagarpreet-chadha ?

@jywarren
Copy link
Member

jywarren commented Aug 4, 2020

Hmm, re: dist file, i wonder if you need to do a clean rebuild with grunt build and readd the dist files so it only includes the content from your /src/ changes in this PR? Thank you both!!!

@jywarren
Copy link
Member

jywarren commented Aug 4, 2020

Also would it be possible to show screenshots (or a gif) of how this works, for documentation's sake and so we all are on the same page? Thank you so much, @Shulammite-Aso !! Great work!

@gitpod-io
Copy link

gitpod-io bot commented Aug 14, 2020

Copy link
Contributor

@sagarpreet-chadha sagarpreet-chadha left a comment

Choose a reason for hiding this comment

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

Hi @Shulammite-Aso , can you rebase your branch with main and then make a new /dist file as Jeff mentioned, then this PR is good to go 💯
Also a screenshot/ GIF if possible 😄

@Shulammite-Aso
Copy link
Collaborator Author

Hi @Shulammite-Aso , can you rebase your branch with main and then make a new /dist file as Jeff mentioned, then this PR is good to go 100
Also a screenshot/ GIF if possible smile

will do this.

@Shulammite-Aso
Copy link
Collaborator Author

Hi @sagarpreet-chadha this is what i'm getting from trying to rebase. Resolved merge conflict by accepting both changes, but running git rebase --continue give me this output:
Screenshot from 2020-08-16 19-30-50

i did use git add to add and commit the changes

@Shulammite-Aso
Copy link
Collaborator Author

Also rebuilding dist file with grunt build is generating the same changes we currently have in the dist file here

@sagarpreet-chadha
Copy link
Contributor

Did you do git add . after resolving conflicts and before doing git rebase --continue?

@Shulammite-Aso
Copy link
Collaborator Author

Did you do git add . after resolving conflicts and before doing git rebase --continue?

Yes @sagarpreet-chadha i did add all the changes

@sagarpreet-chadha
Copy link
Contributor

Okay if you can just pull recent changes from main branch, and resolving conflicts that should be fine then.
Just make sure the commit history is fine using
git log --pretty=oneline
The history should have you commits after last commit of main branch and the final merge commit. In this way we can be sure the commits are right. Thanks!

@Shulammite-Aso
Copy link
Collaborator Author

Hi @sagarpreet-chadha please check this out 😃

Is there anything i can do for the tests to pass? or can we merge it this way?

@sagarpreet-chadha
Copy link
Contributor

Hey @Shulammite-Aso , I reviewed your PR, it is good to go 💯
However 4 tests are failing which arw unrelated to your PR it seems, would you like to debug this?
I think @keshav234156 has opened a PR related to this maybe #602

@gitpod-io
Copy link

gitpod-io bot commented Aug 25, 2020

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.

The publish bar should disappear making more space in small screen.
3 participants