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

CurrencySpace with a non-breaking space #1634

Closed
wants to merge 1 commit into from
Closed

CurrencySpace with a non-breaking space #1634

wants to merge 1 commit into from

Conversation

michelbalzer
Copy link
Contributor

siehe #1628

@michelbalzer michelbalzer changed the title Update Isotope.php CurrencySpace with a non-breaking space Mar 22, 2016
@aschempp
Copy link
Member

Thanks!

Two things:

  1. Why not use  ?
  2. We must only output a HTML character if the output type is HTML (see the method argument).

@aschempp aschempp added the bug label Mar 22, 2016
@aschempp aschempp added this to the 2.3.5 milestone Mar 22, 2016
@michelbalzer
Copy link
Contributor Author

To 1.) Think I saw somewhere else in Isotope that the &# declaration was used. Otherwise there was no specific reason. Both should work.

To 2.) uhm, yes, my bad. The non-html version is used in non-html mails for example, right? We could directly put the character in it.

@michelbalzer
Copy link
Contributor Author

As I researched:   is faster ;)

@aschempp
Copy link
Member

The non-html version is used in non-html mails for example, right? We could directly put the character in it.

Correct, that's for plain text mails for example. Can you update the PR?

@michelbalzer
Copy link
Contributor Author

Uhm, which one do I have to replace?

@aschempp
Copy link
Member

Personally, I would prefer both.   is easier to read for most users/developers, that should always be more important that theoretical performance. And we also must not use HTML in non-HTML mode.

@aschempp
Copy link
Member

Merged in 4aa8fdc

@aschempp aschempp closed this Apr 27, 2016
@michelbalzer michelbalzer deleted the michelbalzer-patch-currencySpace branch February 7, 2017 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants