-
Notifications
You must be signed in to change notification settings - Fork 28
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
fixing UI. Fixes issue #24 #37
base: master
Are you sure you want to change the base?
Conversation
Let me know if any other change is to be made, |
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.
@siddhishree Looks good. Some shadow effects and a change to a bright colour for font would look better. Also kindly make the input fields less wide.
@yellowwoods12 On it. |
@yellowwoods12 Please review the changes. |
Should I work on this same PR or should I create a separate PR |
@Xtremilicious create a separate PR referencing the same issue. |
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.
@siddhishree code looks good.Please make the changes visible to the codepen implementation that you provided above.
@yellowwoods12 I've already updated the codepen implementation. |
@plxity Please review. |
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.
@siddhishree the input boxes look kinda blurry. Can you fix it?
It's probably coming from transform
property given to .outer_box
@AdityaSrivast if your talking about the city and state boxes I purposely changed their colour a bit, since input cannot be entered in those boxes. |
@plxity @yellowwoods12 @AdityaSrivast I have already made all the changes. Any updates? |
src/index.js
Outdated
@@ -46,41 +46,52 @@ class Pincode extends Component { | |||
} | |||
render() { | |||
return ( | |||
<div style={this.props.Container}> | |||
<div style={this.props.Container} class="outer_box"> |
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.
@siddhishree please use className
instead of class
src/index.js
Outdated
value={this.state.pincode} | ||
id="pincode" | ||
type="number" | ||
style={this.props.pincodeInput} | ||
class="pin" |
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.
Similarly for here and other places in the file
@AdityaSrivast I've made the changes. |
@plxity Please review. |
@plxity Can you please resolve the conflicts for this one and merge it? We can then modify the other one accordingly. |
@siddhishree You have to resolve conflicts from your end. |
Fixes #24
Made Few UI changes.
Also updated the codepen implementation so the demo is visible at:
https://codepen.io/Siddhi5/full/GRJMqWB