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

Feature/react event modal #724

Closed
wants to merge 6 commits into from
Closed

Feature/react event modal #724

wants to merge 6 commits into from

Conversation

NaCl4Ever
Copy link

Description
This fix will convert the event modal over to a react component, and remove the original template code from the application.

Fixes #723

@@ -49,6 +51,26 @@ export default class Home extends React.Component {
let totalPromises = 1;
let numberFinished = 0;

let eventId = urlParamsHandler.getUrlParameter('eventId');
var stateId = urlParamsHandler.getUrlParameter('state');
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit, and I need to update the eslint rules, but can you use const or let instead of var

this.setState({
selectedTownHall: townhall
});
$('.event-modal').modal('show');
Copy link
Collaborator

Choose a reason for hiding this comment

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

We would like to be able to get rid of jquery, so can you re implement this open functionality? Either just in the component, or use https://ant.design/components/modal/ from the ui library we're using (sorry I should have brought this up when we met before).

populateEventModal(townhall)
$('.event-modal').modal('show');
if(this.props.selectTownhall) {
this.props.selectTownhall(townhall);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I haven't tried running this locally yet, but I don't see where this is being cleared out. I would think when you close the modal this should go back to null

Copy link
Author

Choose a reason for hiding this comment

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

It's not, wasn't sure how you would like it, so I'll have it clear on modal close.

<React.Fragment>
{
townhall
? <div className="modal fade event-modal" tabIndex="-1" role="dialog" aria-labelledby="myLargeModalLabel">
Copy link
Collaborator

Choose a reason for hiding this comment

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

for this type of check I like townhall && <></> instead of townhall ? <></> : null

Suggested change
? <div className="modal fade event-modal" tabIndex="-1" role="dialog" aria-labelledby="myLargeModalLabel">
&& <div className="modal fade event-modal" tabIndex="-1" role="dialog" aria-labelledby="myLargeModalLabel">

This pull request was closed.
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.

Refactor Event Modal
2 participants