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

ISSUE 220 - Remove unused CSS 'govuk-button' class from the 'search' submit button #223

Merged

Conversation

iqbalgds
Copy link
Contributor

  • The 'search' submit button had the un-necessary CSS class 'govuk-button'.
  • It a someone uses the gem and they are importing the Design System 'govuk-button' CSS stylesheet,
    then it breaks the styling on the button.

⚠️ Don't forget to update the gem version in the CHANGELOG before merging! When you're ready to release bump version file and generate a tag. ⚠️

@iqbalgds iqbalgds force-pushed the issue-220-fix-remove-class-from-search-submit-button branch from 068da59 to fa194f3 Compare March 23, 2021 17:54
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up @iqbalgds 🙌🏻

@@ -1,3 +1,3 @@
module GovukTechDocs
VERSION = "2.2.1".freeze
VERSION = "2.2.2".freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we tend to do releases separately in this repo, rather than making a change and releasing in the same PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - will update

CHANGELOG.md Outdated
@@ -1,5 +1,9 @@
# Changelog

## 2.2.2

- [#218: Remove unnecessary CSS class on the search submit button](https://github.com/alphagov/tech-docs-gem/pull/223)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [#218: Remove unnecessary CSS class on the search submit button](https://github.com/alphagov/tech-docs-gem/pull/223)
- [#223: Remove unnecessary CSS class on the search submit button](https://github.com/alphagov/tech-docs-gem/pull/223)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot

@iqbalgds iqbalgds force-pushed the issue-220-fix-remove-class-from-search-submit-button branch from fa194f3 to 3be232f Compare March 24, 2021 17:52
…submit button

- The 'search' submit button had the un-necessary CSS class 'govuk-button'.
- It a someone uses the gem and they are importing the Design System 'govuk-button' CSS stylesheet,
  then it breaks the styling on the button.
@iqbalgds iqbalgds force-pushed the issue-220-fix-remove-class-from-search-submit-button branch from 3be232f to 3d17fbc Compare March 24, 2021 17:54
@@ -1,5 +1,9 @@
# Changelog

## 2.2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 'Unreleased' until a new version is released (which might be 2.2.2, but could equally be 2.3) – it also makes it clear to users looking at the changelog that these changes haven't gone out yet.

Copy link
Contributor

@richardTowers richardTowers left a comment

Choose a reason for hiding this comment

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

Looks good to me (although I agree with @36degrees about the title in the changelog, so you should probably fix that before merging 😄)

@iqbalgds iqbalgds merged commit d42f6a9 into master Mar 25, 2021
@iqbalgds iqbalgds deleted the issue-220-fix-remove-class-from-search-submit-button branch March 25, 2021 11:53
kr8n3r added a commit to alphagov/paas-tech-docs that referenced this pull request Mar 25, 2021
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