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

Build sb-admin ourselves for customization #124

Draft
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

mkasberg
Copy link

@mkasberg mkasberg commented Jun 3, 2024

This is an alternative approach to #123. The high-level approach is the same: Include some development tooling from NPM to build styles, and commit the built styles to this repository to serve in production. The difference is that in this PR, I'm continuing to use the sb-admin theme and Bootstrap. This will allow us to work incrementally to tweak the theme to our needs rather than overhauling everything. We can still pursue some cleanup such as tweaking cards, changing colors, or consolidating the navbar, but we can do so incrementally in smaller steps (in later PRs).

For the most part, our build tools match those in sb-admin. The difference is that I've ripped out stuff we don't need (pug, scripts, etc) and I've added some things to run PHP via npm start (so a single command can run everything for local dev).

The scss folder is a verbatim copy of the scss from sb-admin.

The scripts folder is based on sb-admin but includes light customization (for our file locations, etc).

By moving the sb-admin template scss into our project and adding some
code to build scss with npm packages, we'll be able to tweak the theme
and colors as needed.
Running `npm start` will use
[concurrently](https://www.npmjs.com/package/concurrently) to:

 - Build SCSS.
 - Start the PHP server.
 - Watch for changes to assets and PHP files, and respond by rebuilding
   scss when necessary and automatically refreshing the browser.
@JohnRDOrazio
Copy link
Member

The only thing I dislike about checking in external code into the repository, is that it's harder to keep the repo clean and orderly with certain conventions. If there are updates from external repositories that need to be checked in, then the code that is pulled in from outside would need to be reformatted and cleaned up if we want it to get a good "score" with for example CodeFactor. I'd say I've made my own code so much better by implementing CodeFactor, following as many suggestions as possible to make the code readable and follow standards; however every external repo that you check-in will probably not follow the same standards and will cause a lower CodeFactor score.

Do you think a possibility could be to simply fork the sb-admin repository, work on the fork in it's own repository, and then either pull in the results in package.json and a build step OR simply continue as we are now, linking directly to the script and css from a cdn... This last option I don't think is all that bad, it avoids any hassle with build steps and keeps the current repo as clean as possible...

@JohnRDOrazio
Copy link
Member

In any case I'm noticing at least one difference in the public facing results compared to the current development branch:
the ol.carousel-indicators and child lis don't have list-style: none set and so we have numbers showing up next to the carousel indicators.

@JohnRDOrazio
Copy link
Member

Do you think a possibility could be to simply fork the sb-admin repository, work on the fork in it's own repository, and then either pull in the results in package.json and a build step OR simply continue as we are now, linking directly to the script and css from a cdn...

I also think that this option would mean making the work on the fork available for the general public: if anyone else wanted for example the "blue" theme back again, we would be offering a service useful not only for a single repository, but for anyone who wanted the blue theme. And we might even find a few things to fix while working at it, which could be pulled back upstream in the main theme repository.

@mkasberg
Copy link
Author

mkasberg commented Jun 3, 2024

If there are updates from external repositories that need to be checked in, then the code that is pulled in from outside would need to be reformatted and cleaned up if we want it to get a good "score" with for example CodeFactor.

I'm proposing that there are never any "updates" to this "external" code. If we move forward with this draft PR, we'd start treating it as a template rather than a dependency.

I didn't format it yet as this is only a draft, but I'd be happy to make any new code match your CodeFactor settings if we want to move forward with this approach.

Do you think a possibility could be to simply fork the sb-admin repository, work on the fork in it's own repository, and then either pull in the results in package.json and a build step OR simply continue as we are now, linking directly to the script and css from a cdn... This last option I don't think is all that bad, it avoids any hassle with build steps and keeps the current repo as clean as possible...

Yes, I do think this would work. It wasn't my preferred approach because I'd prefer to have the scss in the same repo as the code. But if you prefer the separate repo, I could make that work. If you want to pursue this, could you create a fork of sb-admin owned by @Liturgical-Calendar? I don't have permission to do so.

@JohnRDOrazio
Copy link
Member

I created a fork with a new branch blue-theme, as a way of working at bringing the blue theme back again. Then if there are any other ideas / approaches we can always create other branches to work on. And maybe even send a PR back to the upstream repo if it's anything that could be useful for the general public.
https://github.com/Liturgical-Calendar/startbootstrap-sb-admin/tree/blue-theme

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