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

Introducing external js color picker plugin #369

Merged
merged 6 commits into from
Jul 17, 2018

Conversation

ikedas
Copy link
Member

@ikedas ikedas commented Jul 7, 2018

This PR will replace Sympa's home-made color picker with jQuery MiniColors licensed under MIT license.

This change may reduce traffic by javascript (a very little) and cost for maintenance.

And, it may close issue #334 (Sympa bundles part of code taken from other software package).

sympa-colorpicker

<div class="small-6 medium-3 columns" role="cell">
<input type="text" class="colorPicker" id="error_color" name="error_color"
value="[% error_color %]" data-color="[% error_color %]"
disabled="dsabled" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo s/dsabled/disabled/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@xavierba
Copy link
Contributor

I took a quick look at the commits and didn't found anything obvious beside the typo.
A quick runtime test seems ok, I can change colors, etc...
Once merged, this does indeed close #334.

</p>
<div id="help.Colors" class="secondary callout" data-closable>
<p>
[%|loc%]Use the color editor in order to change defined colors. First select the color you want to change and pick a color,then apply it using the test button. The new color is not really installed but it is used only for your own session. When happy with the different colors you chosen, you may save them in a new static CSS.[%END%]
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing space, s/color,then/color, then/
Typo, s/you chosen/you have chosen/

Copy link
Member Author

Choose a reason for hiding this comment

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

As I took care not to change existing phrases, it would be corrected in translation catalog (see also description in #115).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, what needs to be done here is unclear to me.
Shall it be corrected both here and in the translation catalog ? Or only in the translation catalog (po/sympa/sympa.pot) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed the pointer was unclear.
#115 (comment)

Your change on the site above will be reflected to source distribution in the future release.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

</ol>
<div id="help.ColorChart" class="secondary callout" data-closable>
<p>
[%|loc%]Please note that these desciptions don't cover the exact usage of each color parameter, as it would be far too long to describe. What lies in this table should however give you a correct snapshot of what the color parameters are used for. For more details on rendering, feel free to try changing the colors in your session to see how well that works.[%END%]
Copy link
Contributor

Choose a reason for hiding this comment

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

s/desciptions/descriptions/

Copy link
Member Author

Choose a reason for hiding this comment

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

ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ikedas ikedas added this to the 6.2.36 milestone Jul 14, 2018
@ikedas ikedas merged commit 1fb5a1a into sympa-community:sympa-6.2 Jul 17, 2018
@ikedas ikedas deleted the colorpicker_js branch July 24, 2018 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants