-
Notifications
You must be signed in to change notification settings - Fork 210
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
previewgif overflow #947
previewgif overflow #947
Conversation
@publiclab/is-reviewers |
Can you show a screenshot of the full page? |
Add the tag 😁 |
Let someone else review it as well. 2 reviews are necessary. |
The white panel below the add step one. |
Shall I fix the more modules option??? |
@harshkhandeparkar shall I fix the more modules option or is there any other pull request??? |
#909 more modules has a pull but she is inactive for a week I guess. So I want to make it in a single Pr what do you say??? |
I have asked the contributor in #909 if they will contribute. If there is no reply, I think assigning the same issue to a different first time contributor will be better. What do you think? |
Yeah ok!! |
@jywarren see this. |
@jywarren merge this. |
@jywarren see this!!! |
@harshithpabbati jywarren must be really busy. Also please note that this branch has conflicts so it cannot be merged automatically. |
Oh fine I just seen the conflicts. |
Hi, sorry yes, will be better next week -- had a visit all this week from
some great co-workers and had less reviewing time! Thank you, i tried and
failed to fix this so as soon as it's resolved I'm very happy to merge it,
THANKS!!! :-)
…On Fri, Mar 29, 2019 at 3:17 PM Harshith pabbati ***@***.***> wrote:
Oh fine I just seen the conflicts.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#947 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ9P_apJ0UsKoIhA5NkrAjDtKClI5ks5vbmaygaJpZM4cLxf4>
.
|
@jywarren how about increasing the maintainers group??? it will increase the speed. |
yes but we would need to be quite strict with the 2 reviews policy, and
perhaps start to be more strict with the tests requirement? Thank you!!
…On Fri, Mar 29, 2019 at 8:58 PM Harshith pabbati ***@***.***> wrote:
@jywarren <https://github.com/jywarren> how about increasing the
maintainers group??? it will increase the speed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#947 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ9BQ31BMdwQQ7AOBANBWHFhmarnoks5vbraPgaJpZM4cLxf4>
.
|
Yeah what are the things to do to be a maintainer??? |
examples/demo.css
Outdated
@@ -276,4 +276,18 @@ a.name-header{ | |||
.panel-footer{ | |||
height: 45px; | |||
} | |||
<<<<<<< HEAD |
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.
Please fix conflicts
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.
yeah I am doing them.
@harshkhandeparkar done. |
examples/demo.css
Outdated
.panel-heading{ | ||
height:45px; | ||
} | ||
.panel-footer{ | ||
height: 45px; | ||
} | ||
*/ | ||
#gif{ |
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.
Why is this commented?
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.
@jywarren did it in the bootstrap panel.
@jywarren resolved the conflicts. |
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.
Never modify your main branch. Perfect example for this.
src/modules/Grid/info.json
Outdated
@@ -0,0 +1,5 @@ | |||
{ | |||
"name": "Grid Overlay", |
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.
Oh this is messy. Did you modify your main branch?
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.
oops thats because of checkout i GUESS
Done!!! |
My main branch is very clean. |
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.
✔️
@jywarren merge this!! |
@jywarren I know you are busy. But please try to merge this. So that the UI looks much good. |
Great! Looks good!
I think we'd want someone who is a reviewer: #656 but then also is able to be responsible for the overall architecture of the system; @tech4GT has been great because he built a lot of the underlying systems. I think perhaps before we expand the maintainers group, we need to develop some shared written principles and goals, and maybe some style guidance for coding, that we can all reference, so it's easier to come to consensus, what do you think? We could pick this discussion up in #656 maybe? |
@jywarren don't merge this |
I need to merge main into this. |
I am in the most recent main branch and there is no overflow , although the button is slightly off-positioned! |
Yes, i tried fixing it and improved it a little but didn't do nearly as
good a job as Harshith! So, just tell me when it's ready, thank you!!!
…On Mon, Apr 1, 2019 at 4:13 PM aashna27 ***@***.***> wrote:
I am in the most recent main branch and there is no overflow , although
the button is slightly off-positioned!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#947 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1GZ_3FJupY2SxOjpEEr315Kh18tks5vcmhagaJpZM4cLxf4>
.
|
Fixes #946
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
npm test
@publiclab/is-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!