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

Add component wrapper to more components #7048

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AshGDS
Copy link
Contributor

@AshGDS AshGDS commented Jan 24, 2025

What

  • Adds the component guide to result_card, result-item and result-sections components.
  • Fixes the component guide for result-sections
  • Trello card

Why

Standardising our components to use the component wrapper helper will reduce code, increase standardisation, and improve future feature implementation speed.

Bug discovered

When making this change I realised that the component guide for these component is not rendering any of the CSS. For example: http://127.0.0.1:3010/component-guide/result_card.

I was going to just add add_app_component_stylesheet("result-card") etc. to the components but first I looked further into why the CSS wasn't rendering on the component guide. It seems to be because the code which determines whether to include the stylesheet or not is tied up in Ruby in the outcome_presenter.rb file, which means the CSS stylesheet is only added on outcome pages, and therefore the component guide previews have no stylesheet to use.

def add_app_component_stylesheets?
base_path == "/check-uk-visa" && @node.slug == "outcome-work-y" ||
base_path == "/check-benefits-financial-support" && @node.slug == "results" ||
base_path == "/next-steps-for-your-business" && @node.slug == "results"
end

https://github.com/alphagov/smart-answers/blob/main/app/views/smart_answers/custom_result_full_width.erb#L5-L9

I couldn't think of a simple way of fixing this - do you have any suggestions? I think the way it currently is breaks our component isolation principle, since Ruby logic is deciding whether the CSS stylesheet is added or not - so the component can't just be placed wherever needed?

I spoke to the developers who introduced the change - they did suggest as it's a small amount of CSS we could just go back to having add_app_component_stylesheet("result-card") etc. directly on the components, but I'm guessing that would mean the individual CSS loading might break.

I did look into just passing a boolean to the component previews which would then force them include the stylesheet, but I thought that might look confusing on the component guide previews, as we would have to tell devs not to include this extra boolean when they use the component in production.

@govuk-ci govuk-ci temporarily deployed to smart-answers-pr-7048 January 24, 2025 11:47 Inactive
@AshGDS AshGDS force-pushed the add-component-wrapper-to-more-components branch from c996e21 to e11bab1 Compare January 24, 2025 16:36
@govuk-ci govuk-ci temporarily deployed to smart-answers-pr-7048 January 24, 2025 16:36 Inactive
The result section data was invalid and therefore crashing.
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.

2 participants