Skip to content

Edit cors proxy url in app component#43

Open
atefBB wants to merge 3 commits intoamcquade:masterfrom
atefBB:main
Open

Edit cors proxy url in app component#43
atefBB wants to merge 3 commits intoamcquade:masterfrom
atefBB:main

Conversation

@atefBB
Copy link
Copy Markdown

@atefBB atefBB commented Jul 25, 2021

No description provided.

setFetching((prev) => !prev);
if (event.preventDefault != null)
event.preventDefault();
if (event.preventDefault != null) event.preventDefault();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can change the condition to be if (event.preventDefault)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It would be clear to compare it with null instead of implicitly assume that !

{error ? renderAlert() : <div />}
{!past ? <p>Please enter an RSS feed</p> : <div></div>}
{error ? renderAlert() : null}
{!past ? <p>Please enter an RSS feed</p> : null}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can use && operator !past && <p>Please enter an RSS feed</p>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It just a cleaner way to do it !

const handleClose = (event) => {
if (event.currentTarget.innerText != '')
props.getFeed({target: {elements: {feed_url: {value: event.currentTarget.innerText}}}});
if (event.currentTarget.innerText !== "")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can change the if statement to if (event.currentTarget.innerText)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The same as #43 (comment)

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.

2 participants