If you haven’t watched the video yet, I would recommend doing so. I run through the parts of the codebase we’re focusing our attention on, in this article.
That’s the key when working on an existing project — not a greenfield project. You need to define a focus and stick to it like glue. Otherwise, you’ll find features drag, pull requests (PRs) get huge and timelines aren’t met.
Sure, Piccalilli is in itself more of a product type of project, rather than a typical agency type of project, but the risk of escalation is arguably higher because it’s an internal project. The risk comes from the fact we care deeply about the health of the Piccalilli codebase, making ourselves vulnerable to nerd sniping.
It’s not about skipping the problems, it’s about being organisedpermalink
Instead of dealing with everything as you see it, it’s a good idea to keep a refactoring backlog, or in our case, a refactor type for items in the backlog, which are surfaced in Notion database views.
In that clip, there are various types of elements that we want to improve on Piccalilli. The rule is if you see it, you log it. Then, whenever we start a new run of work on Piccalilli, we start with a refactor.
By starting with refactor work, we can do that while discovery and creative sprints are running.
As you can see, there’s a few weeks from the start to when production development work happens, so there’s plenty of time to clear the decks. By starting with refactoring, you’re tackling the tricky stuff early when you’re at the peak of your energy levels for a project too.
It also helps us qualify design decisions early because the source code is fresh in our minds.
What’s going to need to be refactored for this project?permalink
I’ve spent some time calculating what isn’t working well — mainly because I’m the creator of those things 😅 — and I’ve settled on the following items.
We need to tidy course marketing page variables and experiments
This is what months of experimentation looks like:
- Code language
- ts
const pppEnabled = true; let pppDiscount = pppEnabled ? pppDiscounts.items.find((x) => x.countryCode === countryCode) : null; /* EXPERIMENT I want to see if applying a discount from author promos boosts sales a touch. If it does, we can refactor this into something a bit more well thought out. */ if (!pppDiscount) { /* @ts-expect-error */ pppDiscount = currentPromotions.find((x) => x.utmMedium === utmMedium); } const courseBasePrice = 249; const enrollAnchor = '#enroll'; const coursePrice = `£${courseBasePrice}`; const flowCTALabel = `Take the course for just ${coursePrice}`; const generalCTALabel = 'Let’s get you to a level in development that you never thought would be possible.'; const encouragingStatement = `Join over ${usersCount} others who are taking their CSS skills to the next level`; const dockedCTALabel = encouragingStatement; const pppBanner = pppDiscount ? `Howdy, you get a ${pppDiscount.discount} discount because you’re coming in from ${pppDiscount.countryName}. Use code ${pppDiscount.couponCode} at checkout.` : null; const pppSummary = pppDiscount ? `You get a ${pppDiscount.discount} discount which saves you £${(courseBasePrice * pppDiscount.multiplier).toFixed(2)} because you’re coming in from ${pppDiscount.countryName}. Use code ${pppDiscount.couponCode} at checkout to get Complete CSS for only £${(courseBasePrice - courseBasePrice * pppDiscount.multiplier).toFixed(2)}.` : null; const flowCTASummary = generalCTALabel; const flowCTALink = enrollAnchor; const banner = { icon: 'heart', children: generalCTALabel, variant: 'primary', };
At the very least, we need some in-file documentation that explains what these variables do and how they are used on the marketing pages.
We also need to make sure variables are correctly grouped together. For example flowCTA
-based variables are correct in terms of they power the <FlowCTA>
component we use, but they are split apart and messy. These need to be grouped and you guessed it, documented.
Clean up lazy use of @ts-ignore
Again, this is me… I am not a strong TypeScript developer and I also don’t have much time to do development work. Quite the horror combination, I know.
- Code language
- tsx
{ content.previewFrame && ( <div class="flow"> {content.previewFrame.headline && <Headline {...content.previewFrame.headline} />} {content.previewFrame.lede && <Prose content={content.previewFrame.lede} />} <div class="wrapper"> {/* @ts-ignore */} <PreviewFrame client:visible url={content.previewFrame.url} /> </div> </div> ) }
The problem with a //@ts-ignore
is it’s a bit like a //TODO
comment. We write them with the mindset of “I’ll catch that later” and it is never in fact, caught later. At the very least we should be using @ts-expect-error
because if that particular error stops occurring, TypeScript will throw an error that it’s not needed anymore.
A great time to clean @ts-ignore
, @ts-expect-error
and //TODO
comments up though is at the start of a project — especially if they’re present in files you know are going to be affected.
The page specific CSS is a mess
One of the reasons we chose Astro as our framework was being able to import CSS only when you needed it. For example, every component and region pulls in its own CSS file. If the component was <Button>
, it would start like this.
- Code language
- tsx
import '@repo/css/blocks/button';
That is an alias path to our CSS (in the monorepo). It means button.css
only loads if there are <Button>
components on the page.
We do the same thing for specific page CSS too. Most pages on Piccalilli don’t need this capability, but where there’s quite a lot of deviation from the standard user interface (UI) set, adjustments need to be made.
- Code language
- css
.course-lander { /* Magic number, yes, but getting the sidebar and grid aligned is hard work */ --consistent-sidebar-and-grid-size: var(--course-lander-consistent-sidebar-and-grid-size, clamp(16rem, 33%, 410px)); --signup-summary-max-width: var(--course-lander-signup-summary-max-width, 60ch); padding-block: var(--course-lander-block-padding, var(--space-xl)); } .course-lander:has(.docked-cta) { padding-block-start: var(--course-lander-has-docked-cta-padding-reset, 0); } .course-lander .prose { font-size: var(--course-lander-prose-font-size, var(--size-step-1)); } .course-lander .prose p, .course-lander .prose ol, .course-lander .prose ul { max-width: var(--course-lander-prose-max-width, 75ch); } .course-lander .prose { --flow-space: var(--course-lander-prose-flow-space, var(--space-l-xl)); } .course-lander .grid[data-layout='thirds'] { --grid-placement: auto-fill; --grid-min-item-size: var(--consistent-sidebar-and-grid-size); } .course-lander .sidebar:not([data-direction]) { --sidebar-target-width: var(--consistent-sidebar-and-grid-size); --gutter: var(--course-lander-sidebar-gutter, var(--space-l) var(--space-4xl)); } .course-lander__intro .sidebar .wrapper { padding: var(--course-lander-intro-sidebar-wrapper-padding, 0); } .course-lander__intro h3 { font-size: var(--course-lander-intro-h3-size, var(--size-step-5)); } .course-lander__intro .flow-cta { --flow-space: var(--course-lander-flow-cta-flow-space, var(--space-2xl)); } .course-lander__intro .banner, .course-lander__user-form, .course-lander .product-details { --flow-space: var(--course-lander-multi-flow-space-reset, 0); } .course-lander__faq { --flow-space: var(--course-lander-faq-flow-space, var(--space-l-xl)); } .course-lander__closing-cta { --flow-space: var(--course-lander-closing-cta-flow-space, var(--space-4xl)); } .course-lander__intro-action { margin-block-start: var(--course-lander-intro-action-top-margin, var(--space-m)); } .course-lander__intro-action + .banner { padding-inline: 0; } .course-lander .wrapper:has(.praise:not([data-praise-variant])) { margin-block-start: var(--course-lander-default-praise-top-margin, var(--space-4xl)); } .course-lander:has([id='enroll']:target) .docked-cta { display: none; }
Could you tell me what’s going on here? I can’t and and I wrote it. The file started as complete-css.css
and when we launched the initial marketing pages for Mindful Design and JavaScript for Everyone, we normalised everything to be course-lander.css
instead.
What we didn’t do though is audit the CSS and qualify what everything does. The argument for not doing that was all three course marketing pages were going to be identical in terms of UI. We were focusing on the project at hand.
That’s very much not going to be the case on this project though — all three will be completely unique — so the time has come to go through this CSS, determine what each block is doing and then you guessed it: comment it within an inch of its life.
I can hear one or two of you thinking “why do that if you know it’s going to be changed?“. It’s a valid question! What we need to achieve here is confidence when code will be deleted. It’s all about smoothing the path as much as possible, so production-level CSS authoring is fully focused on making stuff look good and maintaining the already high levels of flexibility. By documenting first, we’ll by proxy, enable that smooth working.
I’d say our CSS is in really good shape overall (you’d hope so wouldn’t you?) but even then, deleting CSS can be quite scary if you don’t smooth the path first. By qualifying everything in course-lander.css
either Leanne or I can confidently delete stuff at will, if we need to.
Organise, qualify, regroup and get on with the workpermalink
The next part to do is go through the existing refactor backlog items and determine what could be done in this project. All of this work should be in a separate, smaller PR to the main production work.
The reason for that is we want focused PRs rather than huge PRs. It makes the reviewer’s life a lot easier and more likely they’ll actually analyse the code. If that’s not happening, you’re doing PRs for PR-sake, rather than what they should be for: working together to make sure code is in the best possible shape before it’s merged into the main
branch.
It’s my job as the project lead to do a rough time estimate too to make sure this work doesn’t get in the way of the project. With that in mind and the fact that this project is quite a short timeline, we’ll likely pick up one or two items out of our refactoring backlog to compliment the items I outlined above.
Wrapping uppermalink
Sorry, that ended up being quite long along with a 5 minute video. If you’ve read down this far, you’ve done well.
Let’s move back into the design world on the next post where Jason will explain his initial creative exploration work. It’s much more fun that refactoring!
Sign up to get updates on open working projects
Get a regular round-up of articles we publish for projects so you don’t miss any.
Loading, please wait…
Powered by Postmark - Privacy policy
Our aim with open working projects is to provide progressive movements with excellent web experiences and to also provide the tech community with elite, real world education.
We can only do this with your support though! For us to continue delivering this work, we need to replace our revenue from commercial client projects. Your support with either a pay what you can afford monthly donation, or a one-off donation will go a long way to doing exactly that.
Support Piccalilli
with our transparent Open Collective
Need more convincing? Check out our plans to make open working, work for everyone!