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

48 button #1

Closed
wants to merge 2 commits into from
Closed

48 button #1

wants to merge 2 commits into from

Conversation

divyamrastogi
Copy link

@divyamrastogi divyamrastogi commented May 10, 2018

Fixes UdacityMobileWebScholarship#48
I've removed the CSS file, it wasn't necessary. You can directly use the raised button for our purpose, we don't have to overwrite any of the component-specific styles, we can use them as is.
More documentation about components: https://www.material-ui.com/#/components/raised-button

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream master branch.
  • I have added/updated necessary tests/documentation (if applicable)

Merge this request and then we'll merge your branch with the master.

@divyamrastogi divyamrastogi changed the base branch from master to 48-button May 10, 2018 20:52
@blenderskool
Copy link
Owner

No, I don't want to use the default Raised Button. It is not like the one which is in the design. Even if similar, default raised button won't match the other styles which will be used throughout the app. 😞

@divyamrastogi
Copy link
Author

divyamrastogi commented May 11, 2018 via email

@blenderskool
Copy link
Owner

But I want the ripple and raised effect also of the default button, hence overriding that component.

Creating a custom button component with those features would make it more complicated

@divyamrastogi
Copy link
Author

divyamrastogi commented May 11, 2018 via email

@anurag-majumdar
Copy link

I think we can style the existing button and use that button component throughout the app, @blenderskool has completed that part, it will be great with the design. The characteristics of the elements can be changed via css so it won't be an issue.

@blenderskool
Copy link
Owner

But some styles are not the same. Font weights, sizes, padding, shadows, many things are different

@blenderskool
Copy link
Owner

@divyamrastogi @anurag-majumdar Lets finish the basic working of the app. I don't think we should use the default raised button. If we use it, then we can use all the default components itself... which is very repetitive as of now until the new material design comes for web.

@blenderskool
Copy link
Owner

Closing this request since we should focus on finishing the development of the app.

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.

3 participants