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

Fix service name test #1462

Merged
merged 2 commits into from
Jul 7, 2022
Merged

Fix service name test #1462

merged 2 commits into from
Jul 7, 2022

Conversation

BenSurgisonGDS
Copy link
Contributor

Allows the serverName in the config file to be changed and the change detected correctly when running the kit as a package.

@BenSurgisonGDS BenSurgisonGDS self-assigned this Jul 7, 2022
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-fix-config-jmyddw July 7, 2022 14:43 Inactive
@BenSurgisonGDS BenSurgisonGDS requested a review from a team July 7, 2022 14:51
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-protot-fix-config-jmyddw July 7, 2022 14:51 Inactive
Copy link
Contributor

@nataliecarey nataliecarey left a comment

Choose a reason for hiding this comment

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

I want to keep looking into the reasons behind needing the two different selectors but I don't think that needs to hold this up.

@nataliecarey
Copy link
Contributor

I've found the reason why it needs two different selectors (route cause of the issue). govuk-frontend has a template.njk that includes:

{% if params.serviceName %}
    {% if params.serviceUrl %}
      <a href="{{ params.serviceUrl }}" class="govuk-header__link govuk-header__service-name">
        {{ params.serviceName }}
      </a>
    {% else%}
      <span class="govuk-header__service-name">
        {{ params.serviceName }}
      </span>
    {% endif %}
    {% endif %}

Therefore the presence or absence of serviceUrl makes the difference. Running the kit as we currently recommend makes use of the layout.html which includes the header as:

{% block header %}
  {# Set serviceName in config.js. #}
  {{ govukHeader({
    homepageUrl: "/",
    serviceName: serviceName,
    serviceUrl: "/",
    containerClasses: "govuk-width-container"
  }) }}
{% endblock %}

(note the hard-coded forward slash as the URL)

I'm just trying to see why the same thing isn't happening when it's running as a module.

@BenSurgisonGDS BenSurgisonGDS merged commit 9d6eda5 into main Jul 7, 2022
@BenSurgisonGDS BenSurgisonGDS deleted the fix-config-service-name-test branch July 7, 2022 15:41
@BenSurgisonGDS BenSurgisonGDS linked an issue Jul 22, 2022 that may be closed by this pull request
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.

Increase Integration test coverage for prototype kit
3 participants