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

chore: move plh-specific components to plh components folder #2485

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

jfmcquade
Copy link
Collaborator

@jfmcquade jfmcquade commented Oct 25, 2024

PR Checklist

  • PR title descriptive (can be used in release notes)

Description

Follow-up to #2478.

  • Moves two components into the packages/components/plh folder created in that PR, updating the relevant module declaration and component mapping
  • Refactors the template pipes into a module, which can then be imported into multiple other modules
  • Renames and tidies the component files related to parent-point-box

The code for these plh components is messy and includes hardcoded paths to assets. I don't think refactoring them is a priority right now, and I'm not sure if we plan to continue actively using them.

@FaithDaka – I think it's worth saying for your benefit that the two components that are now in packages/components/plh as of this PR are not good examples of components in themselves! They just happen to be two components that are PLH-specific so I've moved them over. You shouldn't look at the content of those files for an example of how to write component files (instead refer to packages/components/demo/basic). However, how they are configured and declared in packages/components/plh/index.ts is relevant for adding any new PLH components.

Git Issues

Closes #

Screenshots/Videos

Updated comp_parent_point_counter:

Screenshot 2024-10-25 at 17 18 13 Screenshot 2024-10-25 at 17 11 08

comp_parent_point_box. This component sheet doesn't make sense to me, and some assets are missing, but for the purposes of this PR, what is relevant is that this template displays the same on this PR branch as it does on the debug web preview, indicating that the component is being made available to the template authoring as expected

Screenshot 2024-10-25 at 17 17 47

Sorry, something went wrong.

@github-actions github-actions bot added the maintenance Core updates, refactoring and code quality improvements label Oct 25, 2024
Copy link
Member

@chrismclarke chrismclarke left a comment

Choose a reason for hiding this comment

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

Thanks for tidying this up and fixing the component demos a bit

I've added one minor tidying commit a841bcc (given all the pipes were declared in the pipe index felt it made sense to also just export the module from there instead of a separate file), but otherwise I'd say good to go.

I confirmed both the plh component were working and a few others that use pipes, so I'd say all good.

@jfmcquade jfmcquade merged commit aeb2aa4 into master Oct 28, 2024
6 checks passed
@jfmcquade jfmcquade deleted the chore/plh-components-folder branch October 28, 2024 12:25
@jfmcquade jfmcquade restored the chore/plh-components-folder branch October 28, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Core updates, refactoring and code quality improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants