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

input text is too close to edges #530

Closed
bhavayAnand9 opened this issue Dec 5, 2018 · 19 comments
Closed

input text is too close to edges #530

bhavayAnand9 opened this issue Dec 5, 2018 · 19 comments

Comments

@bhavayAnand9
Copy link
Member

bhavayAnand9 commented Dec 5, 2018

Please describe the problem (or idea)

As described in this comment, due to the form-class in demo.css the fields in text input are now too close to the edges.

What happened just before the problem occurred? Or what problem could this idea solve?

using .form-control[type="range"] might solve the problem

What did you expect to see that you didn't?

Too close:

image

OK:

Please show us where to look

https://github.com/publiclab/image-sequencer/blob/main/examples/demo.css#L45

What's your PublicLab.org username?

bhavayanandcse

This can help us diagnose the issue:

Browser, version, and operating system

Many bugs are related to these -- please help us track it down and reproduce what you're seeing!


Thank you!

Your help makes Public Lab better! We deeply appreciate your helping refine and improve this site.

To learn how to write really great issues, which increases the chances they'll be resolved, see:

https://publiclab.org/wiki/developers#Contributing+for+non-coders

@welcome
Copy link

welcome bot commented Dec 5, 2018

Thanks for opening your first issue here! Please follow the issue template to help us help you 👍🎉😄
If you have screenshots to share demonstrating the issue, that's really helpful! 📸 You can make a gif too!

@gitmate gitmate bot added the bug label Dec 5, 2018
@gitmate
Copy link

gitmate bot commented Dec 5, 2018

GitMate.io thinks the contributors most likely able to help are @oorjitchowdhary, and @GitMate[bot].

Possibly related issues are #431 (Text overlay), #383 (Input/Output mismatch on applying edge-detect), #459 (Close-parentheses in Dynamic module input get lost when sourced from URL), #166 (Edge detection module), and #389 (When the input is larger make it as textarea).

1 similar comment
@gitmate
Copy link

gitmate bot commented Dec 5, 2018

GitMate.io thinks the contributors most likely able to help are @oorjitchowdhary, and @GitMate[bot].

Possibly related issues are #431 (Text overlay), #383 (Input/Output mismatch on applying edge-detect), #459 (Close-parentheses in Dynamic module input get lost when sourced from URL), #166 (Edge detection module), and #389 (When the input is larger make it as textarea).

@bhavayAnand9
Copy link
Member Author

bhavayAnand9 commented Dec 5, 2018

it can be a good first-timer-only task.

@kevinzluo
Copy link

The changes should be made here:

.form-control {
padding: 0px 0px;
}

DIFF:

- .form-control {
+ .form-control[type="range"] {
     padding: 0px 0px; 
  } 

@bhavayAnand9
Copy link
Member Author

@jywarren It's a month old issue and very easy to fix. Can you assign it a fto or first-timers-tag? Thanks

@jywarren
Copy link
Member

Done!


This has been marked as a good candidate for becoming a first-timers-only issue like these, meaning that it's simple, self-contained, and with some extra formatting, could be a great entry point for a new contributor. If you're familiar enough with this code, please consider reformatting or reposting it as a first-timers-only issue, and then ping @publiclab/reviewers to get it labelled. Or, if this is not your first time, try to solve it yourself!

@sashadev-sky
Copy link
Member

I will double check @kevinzluo's solution and repost this as a FTO in the next few hours :) @jywarren @bhavayAnand9

@sashadev-sky
Copy link
Member

@bhavayAnand9 @kevinzluo @jywarren I think this issue may be outdated as the UI looks like this now:
http://sequencer.publiclab.org/examples/#steps=contrast{}
screen shot 2019-01-16 at 12 11 15 am

Please confirm.

@bhavayAnand9
Copy link
Member Author

@sashadev-sky other modules like crop which uses text inputs instead of range based have still this issue.
screenshot 2019-01-16 at 16 58 55

@sashadev-sky
Copy link
Member

sashadev-sky commented Jan 17, 2019

@bhavayAnand9 Sorry I missed that! I went through a few other ones and they were converted to the new UI so I figured they all might be. Is there a reason we changed some and not others?

Would it be preferable to change all of them to the new UI for consistency? If so, I can open up FTOs regarding that.

@bhavayAnand9
Copy link
Member Author

@sashadev-sky I don't think we can change all modules because some modules like white-balance can take input from 0-40k so it'll be difficult to adjust that input on a slider. I guess slider method is more suitable with modules having one input and having a range between 1-100. What do you think?

@sashadev-sky
Copy link
Member

@bhavayAnand9 good point! I don't think it matters how many inputs there are just that the range is small enough to be slider friendly as you mentioned. I will open an FTO for your recommended CSS change :)

@sashadev-sky
Copy link
Member

@bhavayAnand9 My branch is updated but I think there might be something wrong with my setup. Can you please advise? I have never used grunt before but I ran grunt serve and the demo.css file I am seeing in the Network panel of Chrome dev tools seems to be an old version... it doesn't have a lot of the styles on it

for me it is showing:
screen shot 2019-01-18 at 6 57 22 am

@bhavayAnand9
Copy link
Member Author

@sashadev-sky Haha! You had forked a wrong repo. You forked it from jywarren/image-sequencer instead of publiclab/image-sequencer. Jywarren's repo is 159 commits behind.

@jywarren
Copy link
Member

jywarren commented Jan 18, 2019 via email

@sashadev-sky
Copy link
Member

@bhavayAnand9 🙈 🙈
I will fix this and open after the weekend

@bhavayAnand9
Copy link
Member Author

Thanks!

@sashadev-sky
Copy link
Member

Closing this issue as I have opened an FTO from it #692

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants