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

Notes to 1.2.4 release #153

Closed
chemerisuk opened this Issue Aug 29, 2010 · 5 comments

Comments

Projects
None yet
2 participants
@chemerisuk

Hi, there are some issues:

  • typo in toolbox.history script: need to remove line 94;

    - typo in tools.validator script: quotes are not required in the name attribute selector at line 329.

    - tools.scrollable: you may use more simple $.noop syntax to create an empty funciton.

Thanks

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 10, 2010

Contributor
  1. removed that line. here is the commit:

http://github.com/jquerytools/jquerytools/commit/d02ed8283553e40196dfeb6e662623fbd6bca01e

that was added by some of the contributors. never tested that. I'm sure there is some scenario/browser where that line is needed. I guess I need to further investigate this.

  1. also added there because of user feedback. I'm sure some people use names with a space character. Not recommended but there are many kind of users.
  2. this would require jquery 1.4 and i don't want it just now.
Contributor

tipiirai commented Sep 10, 2010

  1. removed that line. here is the commit:

http://github.com/jquerytools/jquerytools/commit/d02ed8283553e40196dfeb6e662623fbd6bca01e

that was added by some of the contributors. never tested that. I'm sure there is some scenario/browser where that line is needed. I guess I need to further investigate this.

  1. also added there because of user feedback. I'm sure some people use names with a space character. Not recommended but there are many kind of users.
  2. this would require jquery 1.4 and i don't want it just now.
@chemerisuk

This comment has been minimized.

Show comment
Hide comment
@chemerisuk

chemerisuk Sep 10, 2010

For me it's very strange to use space in value of name attribute, but of cause nothing is possible.

Another issue I want to propose is for validator script. It uses now "span" tag to wrap a error message. I believe it's very rare situation to display more than one message at the same line. So for a better markup did changes in my local copy to use "p" instead of "span". What do you think?

For me it's very strange to use space in value of name attribute, but of cause nothing is possible.

Another issue I want to propose is for validator script. It uses now "span" tag to wrap a error message. I believe it's very rare situation to display more than one message at the same line. So for a better markup did changes in my local copy to use "p" instead of "span". What do you think?

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 10, 2010

Contributor

it's strange. should never use a space in attribute name or any other weird characters. but people are people.

the message container is specified in configuration. by default it's

message: '

',

inside this container we are adding individual error messages. they are wrapped inside a span element. i would say this is a solid system.

Contributor

tipiirai commented Sep 10, 2010

it's strange. should never use a space in attribute name or any other weird characters. but people are people.

the message container is specified in configuration. by default it's

message: '

',

inside this container we are adding individual error messages. they are wrapped inside a span element. i would say this is a solid system.

@chemerisuk

This comment has been minimized.

Show comment
Hide comment
@chemerisuk

chemerisuk Sep 10, 2010

Correct, but is it regular situation when several error messages (wrapped in "span") should be displayed at the same line? I made a decision it isn't, and because of "span" tag by default doesn't have "display: block" in css, changed it on "p".

Correct, but is it regular situation when several error messages (wrapped in "span") should be displayed at the same line? I made a decision it isn't, and because of "span" tag by default doesn't have "display: block" in css, changed it on "p".

@tipiirai

This comment has been minimized.

Show comment
Hide comment
@tipiirai

tipiirai Sep 11, 2010

Contributor

hello,

ok. this is now changed. here is the patch:

http://github.com/jquerytools/jquerytools/commit/7fa96acf0e02d8cf5919ee61354c896ee51d1bc5

I'm sure I'll get a lot of feedback that the error messages look different now. this is because span elements does not have any margin by default but p elements does.

Contributor

tipiirai commented Sep 11, 2010

hello,

ok. this is now changed. here is the patch:

http://github.com/jquerytools/jquerytools/commit/7fa96acf0e02d8cf5919ee61354c896ee51d1bc5

I'm sure I'll get a lot of feedback that the error messages look different now. this is because span elements does not have any margin by default but p elements does.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment