Front-end education for the real world. Since 2018.





A look at the codebase and what needs refactoring

Andy Bell

Project: Course Brand Development

Sprint: Sprint 1 - Planning & creative exploration


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.

A Notion board titled 'Backlog' under the 'Refactor' tab shows a list of tasks with columns for assignment, due date, and type. Tasks include items like removing inline padding for small viewports, merging variables, and updating excluded categories. All tasks are tagged as 'Refactor'.

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.

5 blocks made from a red striped pattern. They're laid out like a Gantt chart which visualises each block following the other. They are labeled "Planning & creative exploration", "Creative production", "Prototypes & technical planning", "Technical production" and "Quality assurance & final details"

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.descriptor}. 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.descriptor}. 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!

Enjoyed this article? Consider leaving a tip on Open Collective

Support Piccalilli

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

and see how we’re planning to make a genuine, positive impact