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

RealmInputScreen [nfc]: Inline SmartUrlInput #5572

Merged
merged 9 commits into from
Dec 2, 2022

Conversation

chrisbobbe
Copy link
Contributor

No description provided.

@chrisbobbe chrisbobbe requested a review from gnprice November 28, 2022 20:54
@chrisbobbe chrisbobbe force-pushed the pr-inline-smart-url-input branch from 1617e0d to d7723c5 Compare November 28, 2022 20:54
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks @chrisbobbe for this cleanup!

All looks good -- just one small further cleanup opportunity noted below. Please merge at will.

Comment on lines 90 to 92
const handleRealmChange = useCallback(value => {
setRealmInputValue(value);
}, []);
Copy link
Member

Choose a reason for hiding this comment

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

Can simplify this (in a followup commit) to just setRealmInputValue -- just refer to that directly instead of adding the name handleRealmChange.

@chrisbobbe chrisbobbe force-pushed the pr-inline-smart-url-input branch from d7723c5 to c93923e Compare December 2, 2022 05:47
@chrisbobbe chrisbobbe merged commit c93923e into zulip:main Dec 2, 2022
@chrisbobbe
Copy link
Contributor Author

Thanks for the review!

All looks good -- just one small further cleanup opportunity noted below. Please merge at will.

Merged, with that cleanup commit.

@chrisbobbe chrisbobbe deleted the pr-inline-smart-url-input branch December 2, 2022 05:48
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