-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: add origin selector #116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's working nice. I just want to make sure that it is expected to be able to change the origin in any place and when we do it, go back to the revisions initial page, right?
return ( | ||
<SelectUI {...propsWithoutChildren}> | ||
<SelectTrigger className="w-auto rounded-full border-2 border-dimGray px-6 py-4 text-base font-medium text-dimGray"> | ||
<SelectValue placeholder="Theme" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add a better placeholder and make it use i18n?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually this shouldn't be here, i was using it for testing purposes. Nice catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove it?
|
||
return ( | ||
<div className="flex items-center"> | ||
<span className="mr-4 text-base font-medium text-dimGray">Origin</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use intl
<div className="flex w-full flex-row items-center justify-between"> | ||
<span className="text-2xl"> | ||
<div className="flex flex-row items-center justify-between"> | ||
<span className="mr-14 text-2xl"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just make sure it will be aligned with other content, because of the changes we are making
d7009b0
to
381a0d0
Compare
return ( | ||
<SelectUI {...propsWithoutChildren}> | ||
<SelectTrigger className="w-auto rounded-full border-2 border-dimGray px-6 py-4 text-base font-medium text-dimGray"> | ||
<SelectValue placeholder="Theme" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove it?
381a0d0
to
defa4cd
Compare
defa4cd
to
c64d035
Compare
close: #78