-
Notifications
You must be signed in to change notification settings - Fork 1
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: Filter SDKs with / #306
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.
@k3llymariee This probably requires running locally to see how the filter works since you can filter to find a something, filter too much to find nothing; close the filter; and select an SDK, go forward, then come back to have the selected SDK still highlighted.
l.Title = "Select your SDK:\n" | ||
l.FilterInput.PromptStyle = lipgloss.NewStyle() | ||
|
||
l.Title = "Select your SDK:" |
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.
Removed the newline and added a margin bottom to work with the title text and the filter title text.
@@ -61,6 +64,13 @@ func NewChooseSDKModel(selectedIndex int) tea.Model { | |||
} | |||
} | |||
|
|||
// initSDKs sets the index of each SDK based on place in list. | |||
func initSDKs() { |
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.
Do this programmatically instead of explicitly setting an index on each SDK. We use the index to keep track of which SDK was selected so it is highlighted when the user goes back to the list.
{canonicalName: "java", displayName: "Java", kind: serverSideSDK}, | ||
{canonicalName: "dotnet-server", displayName: ".NET (server-side)", kind: serverSideSDK}, | ||
{canonicalName: "js", displayName: "JavaScript", kind: clientSideSDK}, | ||
{ |
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.
Split these out for consistency.
@@ -202,7 +202,9 @@ func (m ContainerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { | |||
m.currentStep += 1 | |||
shouldSendTrackingEvent = true | |||
default: | |||
// ignore other messages | |||
// delegate other messages |
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.
We need to pass the list messages from the container down into the show SDK model.
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.
🥳
@@ -72,11 +82,17 @@ func (m chooseSDKModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { | |||
switch msg := msg.(type) { | |||
case tea.KeyMsg: | |||
switch { | |||
case msg.String() == "/": |
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.
we should also add this to the help keys so it's more discoverable
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.
Done. I added a screenshot to see it show up only on the choose SDK page.
Can filter the SDK names.
Requirements
Related issues
Provide links to any issues in this repository or elsewhere relating to this pull request.
Describe the solution you've provided
Provide a clear and concise description of what you expect to happen.
Describe alternatives you've considered
Provide a clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context about the pull request here.