Skip to content
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

Move more examples from SelectNextPage to example files #1563

Merged
merged 1 commit into from
May 21, 2024

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented May 21, 2024

This PR migrates a few more examples from Selectnext page to the exampleFile capability introduced in #1558

While working on this, I have found a few nice-to-have things that we could eventually implement:

  • Allow passing props to example files, so that the same example can be reused for very similar cases.
  • Allow certain parts of the code snippet to be hidden, so that we can use relatively complex examples, but avoid distractions due to the boilerplate code.
    This could be done by introducing an opening/closing comment pattern that can be used to identify blocks and replace them with, let's say, a comment like // [...].

Comment on lines +7 to +8
import SelectNextInInputGroup from '../../../examples/select-next-in-input-group';
import SelectNextWithManyOptions from '../../../examples/select-next-with-custom-options';
Copy link
Contributor Author

@acelaya acelaya May 21, 2024

Choose a reason for hiding this comment

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

Explicitly importing some examples to reuse them in some cases where the code snippet is not that important, but we want to show how it looks when rendered. This way we don't duplicate that much code.

This could be addressed differently if we had a mechanism to pass props to example files loaded via exampleFile, but that is not the case at the moment.

| 'listboxClasses'
| 'disabled'
| 'right'
| 'listboxAsPopover'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This local SelectExample component is still used in some cases, but I have removed all props which are no longer needed because they have been moved to example files.

@acelaya acelaya requested a review from robertknight May 21, 2024 09:41
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

Looks good. Duplicating the example source to show variations in one prop is unfortunate, but the alternative is parametrizing the example which would make the individual examples more complex, so the duplication is probably the nicer thing for consumers of the library.

@acelaya acelaya merged commit 2a2c6ab into main May 21, 2024
4 checks passed
@acelaya acelaya deleted the more-example-files branch May 21, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants