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

Sort components by name #114

Merged
merged 2 commits into from Jan 3, 2018
Merged

Sort components by name #114

merged 2 commits into from Jan 3, 2018

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jan 3, 2018

Components were being ordered by their filename but displayed their name in the component list. For some components such as “Form label”
(label.html.erb), the component wasn’t appearing where expected.

  • Always sort the list of components by name

Before

screen shot 2018-01-03 at 11 35 17

After

screen shot 2018-01-03 at 11 35 05

Components were being ordered by their filename but displayed their
name in the component list. For some components such as “Form label”
(label.html.erb), the component wasn’t appearing where expected.

* Always sort the list of components by name
@fofr
Copy link
Contributor Author

@fofr fofr commented Jan 3, 2018

Weird error on this, unrelated to the change:

Uglifier::Error: Unexpected token: keyword (const). To use ES6 syntax, harmony mode must be enabled with Uglifier.new(:harmony => true).

Guessing it relates to:

https://github.com/alphagov/govuk_publishing_components/blob/order-by-name/app/assets/javascripts/govuk_publishing_components/visual-regression.js#L5

cc @nickcolley @vanitabarrett

@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Jan 3, 2018

That is odd it wasn't raised before hand, we should be OK to change that to var for now.

Fixes error with Uglifier as we have not set a Harmony preference.
@nickcolley
Copy link
Contributor

@nickcolley nickcolley commented Jan 3, 2018

🚢 👍

@fofr fofr merged commit 54b760f into master Jan 3, 2018
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@fofr fofr deleted the order-by-name branch Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.