-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Improve Look of SelectApp and SelectComponent #15674
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
Changes from 6 commits
e2b020a
4eb4748
7b7b2f6
5e10197
3dab83f
8940396
6f7b748
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,7 +25,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } = useApps({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| q, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { Option } = components; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { Option, SingleValue } = components; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 28 in packages/connect-react/src/components/SelectApp.tsx
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const selectedValue = apps?.find((o) => o.name_slug === value?.name_slug) || null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Select | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| instanceId={instanceId} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -52,14 +53,30 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Option> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SingleValue: (singleValueProps) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SingleValue {...singleValueProps}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div style={{ display: "flex", gap: 10, alignItems: "center" }}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 58 in packages/connect-react/src/components/SelectApp.tsx
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <img | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| src={`https://pipedream.com/s.v0/${singleValueProps.data.id}/logo/48`} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| style={{ height: 24, width: 24 }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Check failure on line 61 in packages/connect-react/src/components/SelectApp.tsx
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alt={singleValueProps.data.name} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span style={{ whiteSpace: "nowrap" }}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {singleValueProps.data.name} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </SingleValue> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion LGTM! Enhanced visual presentation. The SingleValue component implementation improves the visual appearance with consistent styling. However, the formatting needs adjustment: Apply this diff to fix the formatting: SingleValue: (singleValueProps) => (
<SingleValue {...singleValueProps}>
- <div style={{ display: "flex", gap: 10, alignItems: "center" }}>
+ <div style={{
+ display: "flex",
+ gap: 10,
+ alignItems: "center",
+ }}>
<img
- src={`https://pipedream.com/s.v0/${singleValueProps.data.id}/logo/48`}
- style={{ height: 24, width: 24 }}
+ src={`https://pipedream.com/s.v0/${singleValueProps.data.id}/logo/48`}
+ style={{
+ height: 24,
+ width: 24,
+ }}
alt={singleValueProps.data.name}
/>
- <span style={{ whiteSpace: "nowrap" }}>
+ <span style={{
+ whiteSpace: "nowrap",
+ }}>
{singleValueProps.data.name}
</span>
</div>📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Lint Code Base[failure] 58-58: [failure] 58-58: [failure] 58-58: [failure] 58-58: [failure] 61-61: [failure] 61-61: [failure] 61-61: [failure] 64-64: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IndicatorSeparator: () => null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options={apps || []} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getOptionLabel={(o) => o.name || o.name_slug} // TODO fetch initial value app so we show name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getOptionValue={(o) => o.name_slug} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={value} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={selectedValue} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={(o) => onChange?.((o as AppResponse) || undefined)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onInputChange={(v) => setQ(v)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onInputChange={(v) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if(v) setQ(v) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve input change handler. The current implementation might miss important input changes. Apply this diff to handle empty string inputs: - onInputChange={(v) => {
- if(v) setQ(v)
- }}
+ onInputChange={(v) => setQ(v || "")}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isLoading={isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Fix formatting according to linting rules.
The component destructuring needs formatting adjustments.
Apply this diff to fix the formatting:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: Lint Code Base
[failure] 28-28:
Expected a line break after this opening brace
[failure] 28-28:
Expected a line break before this closing brace
🪛 GitHub Actions: Pull Request Checks
[error] 28-28: Expected a line break after this opening brace (object-curly-newline).