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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change icon for Surveys #2433

Merged
merged 2 commits into from Jan 9, 2018

Conversation

Projects
None yet
3 participants
@SeanPrashad
Contributor

SeanPrashad commented Dec 29, 2017

馃帺 What? Why?

Modified the contents of decidim-surveys/app/assets/images/decidim/surveys/icon.svg to reflect the same .svg found here.

馃搶 Related Issues

馃搵 Subtasks

  • N/A

馃摲 Screenshots (optional)

  • N/A

馃懟 GIF

  • N/A

@SeanPrashad SeanPrashad changed the title from Fix 2390 - Change icon for Surveys to Change icon for Surveys Dec 29, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Dec 29, 2017

Codecov Report

Merging #2433 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2433      +/-   ##
==========================================
- Coverage   98.68%   98.68%   -0.01%     
==========================================
  Files        1294     1294              
  Lines       30199    30172      -27     
==========================================
- Hits        29802    29775      -27     
  Misses        397      397

codecov bot commented Dec 29, 2017

Codecov Report

Merging #2433 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2433      +/-   ##
==========================================
- Coverage   98.68%   98.68%   -0.01%     
==========================================
  Files        1294     1294              
  Lines       30199    30172      -27     
==========================================
- Hits        29802    29775      -27     
  Misses        397      397
@SeanPrashad

This comment has been minimized.

Show comment
Hide comment
@SeanPrashad

SeanPrashad Dec 31, 2017

Contributor

@xabier I've switched over the icon but I wasn't sure whether to keep the original file and change it's contents or delete it and copy of the new file. I went with the former but if I need to change it, please let me know!

Contributor

SeanPrashad commented Dec 31, 2017

@xabier I've switched over the icon but I wasn't sure whether to keep the original file and change it's contents or delete it and copy of the new file. I went with the former but if I need to change it, please let me know!

@mrcasals

This comment has been minimized.

Show comment
Hide comment
@mrcasals

mrcasals Jan 8, 2018

Collaborator

@SeanPrashad thanks for your PR!

Surveys is a Decidim feature, and it's registered this way:

Decidim.register_feature(:surveys) do |feature|
feature.engine = Decidim::Surveys::Engine
feature.admin_engine = Decidim::Surveys::AdminEngine
feature.icon = "decidim/surveys/icon.svg"
feature.stylesheet = "decidim/surveys/surveys"

On line 8, we set the path for the icon. This will be used to render menus. If you changed the file name, we would need to change this here.

TLDR: your solution is good, no need to change anything else! 馃槃

Collaborator

mrcasals commented Jan 8, 2018

@SeanPrashad thanks for your PR!

Surveys is a Decidim feature, and it's registered this way:

Decidim.register_feature(:surveys) do |feature|
feature.engine = Decidim::Surveys::Engine
feature.admin_engine = Decidim::Surveys::AdminEngine
feature.icon = "decidim/surveys/icon.svg"
feature.stylesheet = "decidim/surveys/surveys"

On line 8, we set the path for the icon. This will be used to render menus. If you changed the file name, we would need to change this here.

TLDR: your solution is good, no need to change anything else! 馃槃

CHANGELOG entry missing

@mrcasals

This comment has been minimized.

Show comment
Hide comment
@mrcasals

mrcasals Jan 8, 2018

Collaborator

Oh, now that I check, can you add a CHANGELOG entry for this? It should be under UNRELEASED -> Changed, with a text similar to "Updated icon for surveys feature".

Please copy the style of other entries, updating the link to the PR and so 馃槃

Collaborator

mrcasals commented Jan 8, 2018

Oh, now that I check, can you add a CHANGELOG entry for this? It should be under UNRELEASED -> Changed, with a text similar to "Updated icon for surveys feature".

Please copy the style of other entries, updating the link to the PR and so 馃槃

@xabier

This comment has been minimized.

Show comment
Hide comment
@xabier

xabier Jan 8, 2018

Member

@SeanPrashad thanks! sorry I was away on hollidays

Member

xabier commented Jan 8, 2018

@SeanPrashad thanks! sorry I was away on hollidays

@xabier

This comment has been minimized.

Show comment
Hide comment
@xabier

xabier Jan 8, 2018

Member

@mrcasals , give it a merge it all seems completed now! nice job @SeanPrashad 馃憤

Member

xabier commented Jan 8, 2018

@mrcasals , give it a merge it all seems completed now! nice job @SeanPrashad 馃憤

@mrcasals

Wohoooo! 馃帀 Thanks @SeanPrashad 馃榿

@mrcasals mrcasals merged commit b0a1d4a into decidim:master Jan 9, 2018

19 checks passed

ci/circleci: accountability Your tests passed on CircleCI!
Details
ci/circleci: admin Your tests passed on CircleCI!
Details
ci/circleci: api Your tests passed on CircleCI!
Details
ci/circleci: assemblies Your tests passed on CircleCI!
Details
ci/circleci: budgets Your tests passed on CircleCI!
Details
ci/circleci: build_test_app Your tests passed on CircleCI!
Details
ci/circleci: comments Your tests passed on CircleCI!
Details
ci/circleci: core Your tests passed on CircleCI!
Details
ci/circleci: main Your tests passed on CircleCI!
Details
ci/circleci: meetings Your tests passed on CircleCI!
Details
ci/circleci: pages Your tests passed on CircleCI!
Details
ci/circleci: processes Your tests passed on CircleCI!
Details
ci/circleci: proposals Your tests passed on CircleCI!
Details
ci/circleci: surveys Your tests passed on CircleCI!
Details
ci/circleci: system Your tests passed on CircleCI!
Details
ci/circleci: verifications Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codecov/patch Coverage not affected when comparing 7485361...1a421d8
Details
codecov/project 98.68% (-0.01%) compared to 7485361
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment