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

[FRAME-91] Asset File Structure #20

Merged
merged 19 commits into from
Dec 1, 2022
Merged

[FRAME-91] Asset File Structure #20

merged 19 commits into from
Dec 1, 2022

Conversation

dotjon
Copy link
Contributor

@dotjon dotjon commented Nov 17, 2022

This PR will enqueue the admin assets, and change the directory structure to house resources.

src/
++Telemetry/ ← class files here
++++Contracts/
++++++CronJob.php
++++Telemetry.php
++resources/ ← assets here
++++css/
++++images/
++++++Logo.png
++++js/
++views/

Copy link
Contributor

@ChrisMKindred ChrisMKindred left a comment

Choose a reason for hiding this comment

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

We need to start adding docblocks to classes, methods, and filters for new files to help cut down on additional work in a refactoring pass.

I am not sure that there are many options for only loading the js and CSS when needed, but something I'd like us to think about as we continue forward.

src/Telemetry/Admin_Subscriber.php Outdated Show resolved Hide resolved
@dotjon
Copy link
Contributor Author

dotjon commented Nov 28, 2022

@tarecord This PR is ready for re-review. It tackles tickets https://moderntribe.atlassian.net/browse/FRAME-91 and https://moderntribe.atlassian.net/browse/FRAME-96

@dotjon dotjon requested a review from ChrisMKindred November 28, 2022 15:06
Copy link
Contributor

@tarecord tarecord left a comment

Choose a reason for hiding this comment

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

Everything looks so much better! Just a couple of minor things and this should be good.

src/views/exit-interview.php Outdated Show resolved Hide resolved
src/views/exit-interview.php Outdated Show resolved Hide resolved
src/views/optin.php Outdated Show resolved Hide resolved
src/views/optin.php Outdated Show resolved Hide resolved
src/views/optin.php Outdated Show resolved Hide resolved
src/Telemetry/Admin_Resources.php Outdated Show resolved Hide resolved
src/Telemetry/Admin_Subscriber.php Outdated Show resolved Hide resolved
@dotjon dotjon requested a review from tarecord November 29, 2022 09:11
@dotjon dotjon changed the title Asset File Structure [FRAME-83] Asset File Structure Nov 29, 2022
@dotjon dotjon changed the title [FRAME-83] Asset File Structure [FRAME-91] Asset File Structure Nov 29, 2022
Copy link
Contributor

@tarecord tarecord 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! 🚢 💨

Copy link
Contributor

@ChrisMKindred ChrisMKindred 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.

@dotjon dotjon merged commit a0da544 into main Dec 1, 2022
@dotjon dotjon deleted the feature/asset-structure branch December 1, 2022 10:09
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.

3 participants