Skip to content

Web: make SelectResource.tsx component leaner#51698

Merged
kimlisa merged 2 commits intomasterfrom
lisa/move-files
Feb 1, 2025
Merged

Web: make SelectResource.tsx component leaner#51698
kimlisa merged 2 commits intomasterfrom
lisa/move-files

Conversation

@kimlisa
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa commented Jan 31, 2025

part of the improve resource enrollment page work

All this PR does is make the SelectResource.tsx leaner by moving things into own files and directories (so it's easier to work with).

No implementation logic has been changed (just copy and pasta and some renames)

@github-actions github-actions Bot requested review from avatus and kiosion January 31, 2025 08:57
@kimlisa kimlisa added the no-changelog Indicates that a PR does not require a changelog entry label Jan 31, 2025
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from kiosion January 31, 2025 22:57
@kimlisa kimlisa added this pull request to the merge queue Feb 1, 2025
Merged via the queue into master with commit 2004893 Feb 1, 2025
@kimlisa kimlisa deleted the lisa/move-files branch February 1, 2025 00:30
@public-teleport-github-review-bot
Copy link
Copy Markdown

@kimlisa See the table below for backport results.

Branch Result
branch/v17 Create PR

kimlisa added a commit that referenced this pull request Feb 14, 2025
* Web: Move resource specs into resource directory

* Web: move code out into separate files
onSelectResource(resourceSpec);
};

let resourceCardProps: ComponentPropsWithoutRef<'button' | typeof Link>;
Copy link
Copy Markdown
Member

@ravicious ravicious Feb 14, 2025

Choose a reason for hiding this comment

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

This does not work as expected, as Link is an untyped React component, so the props of it are just {[x: string]: any}. I tried removing unnecessary wrapping of Link in #51842 (as in StyledButtonLink can be exported directly as Link) and converting it to .tsx, but got stuck at this.

Ultimately handling the as prop is quite complex. Instead of assigning specific props to a var, perhaps it'd be easier to assign all children of ResourceCard to a separate var. Then the component could end with something like this:

if (something) {
  return <ResourceCard {...sharedProps} foo="bar">{$children}</ResourceCard>
} else if (somethingElse) {
  return <ResourceCard as={Link} {...sharedProps} baz="quux">{$children}</ResourceCard>
}

This way we don't have to deal with providing a type for as at all and type should get inferred correctly.

carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
* Web: Move resource specs into resource directory

* Web: move code out into separate files
github-merge-queue Bot pushed a commit that referenced this pull request Feb 20, 2025
…52176)

* Web: make SelectResource.tsx component leaner (#51698)

* Web: Move resource specs into resource directory

* Web: move code out into separate files

* Web: define discover resource guide ids and add discover user preference field (#51672)

* Add discover resource preferences to user preferences

* Define hard coded guide id consts

* Set guide ids for resources

* Add test

* Address CRs

* Define a new type just for SelectResource.tsx which requires id field

* Wrap discover guide preference "pinned" field inside a proto message (#52017)

This allows us to use "nil" value to mean no discover
guide preferences set (which is used to set default
values in the web UI).

* Web: Add filters and pinning support for Enroll Resources page (#51893)

* Minor changes

- Stop propagation and default behavior when clicking on pin button
- Add small or large icon option for discover icons

* Add pinning guide support

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

Labels

backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/md ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants