Skip to content

Show errors if fetching ParameterPicker suggestions fails#25703

Merged
ravicious merged 4 commits intomasterfrom
ravicious/param-picker-err
May 8, 2023
Merged

Show errors if fetching ParameterPicker suggestions fails#25703
ravicious merged 4 commits intomasterfrom
ravicious/param-picker-err

Conversation

@ravicious
Copy link
Copy Markdown
Member

ParameterPicker used to have no error handling when fetching db usernames. Any errors would be simply swallowed. This is a leftover from the prototype version which this PR addresses. Now the error will show up in a similar fashion to resource search errors in ActionPicker.

ParameterPicker error

Resetting the active item on pick

The first commit is unrelated to the PR but I wanted to squeeze it in.

Before this change, the active item index would not change after picking a filter, making the selection stay in place.

Screen recording of previous behavior
active-item.mov

I don't think is what the typical user expects – if I select a filter, I think I'd rather have the active item reset to the first one – selecting a filter has totally reorganized the results lists.

Without this change it was also possible for the selection to stay the same after selecting an SSH server and switching from selecting a resource to selecting a login.

/**
* IconAndContent is supposed to be used within InteractiveItem & NonInteractiveItem.
*/
export function IconAndContent(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I didn't have better idea for a name. 🤕

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tried to come up with a suggestion and I couldn't either. Sounds like its good enough to me

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved the story from ActionPicker.story.tsx to just results.story.tsx. We could have separate stories for ActionPicker and ParameterPicker but I'm not sure if it makes sense at the moment, we just have a single item from ParameterPicker here. I like being able to see them all at once.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's fine to me.

@ravicious ravicious force-pushed the ravicious/param-picker-err branch from 9408fb0 to 78182ba Compare May 5, 2023 14:45
@ravicious ravicious marked this pull request as ready for review May 5, 2023 14:45
@github-actions github-actions Bot requested review from gzdunek and ryanclark May 5, 2023 14:45
@ravicious ravicious requested review from avatus and removed request for ryanclark May 5, 2023 14:46
/**
* IconAndContent is supposed to be used within InteractiveItem & NonInteractiveItem.
*/
export function IconAndContent(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tried to come up with a suggestion and I couldn't either. Sounds like its good enough to me

@ravicious ravicious enabled auto-merge May 8, 2023 09:39
@ravicious ravicious added this pull request to the merge queue May 8, 2023
Merged via the queue into master with commit b745a82 May 8, 2023
@ravicious ravicious deleted the ravicious/param-picker-err branch May 8, 2023 09:57
@public-teleport-github-review-bot
Copy link
Copy Markdown

@ravicious See the table below for backport results.

Branch Result
branch/v13 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants