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

refactor: change codebase into material-ui #2

Merged
merged 26 commits into from Nov 12, 2019
Merged

Conversation

viinxent
Copy link

Hi everyone!

I did not change any logic on the app. The app works as it is, this branch purpose was to make sure to remove the react-bootstrap dependencies/components and move to material ui one. Also I didn't remove the packages yet for react-bootstrap.

The design is kind of meh as of now, since my main focus was to make it work for material ui.

Problem faced:

There's this warning using the stripe elements with material ui, can't seem to rid of it.
Material UI have this "hackish" way of using stripe elements.

Problem can be seen at this commit: 4d4bebf

See this post/issue from material ui:
mui/material-ui#16037

@viinxent viinxent added enhancement New feature or request refactor labels Nov 11, 2019
@viinxent viinxent self-assigned this Nov 11, 2019
Copy link

@xuatz xuatz left a comment

Choose a reason for hiding this comment

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

Let's apply the changes after the typescript migration.


import { useFormFields } from '../libs/hooksLib';

function StripeInput(props) {
Copy link

Choose a reason for hiding this comment

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

What's happening here?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @xuatz

I was trying to integrate the react-stripe-elements into material-ui unfortunately they don't have a clean way of doing it. One of the author of material-ui created a snippet. TextField component of material ui have a InputProps property which in a way you could override the default component by passing a custom props. It's not actually documented on their web unfortunately.

But they have a topic for it here:
mui/material-ui#16037

const [isLoading, setIsLoading] = useState(true);

useEffect(() => {
async function onLoad() {
Copy link

Choose a reason for hiding this comment

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

why do you need to create new onLoad() every time?

Copy link
Author

Choose a reason for hiding this comment

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

Hey @xuatz

Sorry, I didn't change the logic of the application boilerplate currently. So there are questionable logic in the current state of the app. They will be moved out after I integrated redux/redux-saga


function loadNotes() {
return API.get('notes', '/notes');
}
Copy link

Choose a reason for hiding this comment

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

could move this out of the component

@xuatz xuatz changed the title Refactored react-bootstrap to material ui refactor: change codebase into material-ui Nov 12, 2019
@xuatz xuatz merged commit d3f136c into master Nov 12, 2019
@xuatz xuatz deleted the refact/material-ui branch November 12, 2019 09:42
@xuatz xuatz restored the refact/material-ui branch November 12, 2019 12:08
@viinxent viinxent deleted the refact/material-ui branch November 13, 2019 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
2 participants