-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[OPEN] Move the Project Preferences Dialog variables to the template #3286
Conversation
Open for committers to review. |
<label> | ||
{{PROJECT_SETTING_BASE_URL}}: <input type="text" placeholder="{{PROJECT_SETTING_BASE_URL_HINT}}" value="{{baseUrl}}" class="url" /> | ||
</label> | ||
{{#errorMessage}}<div class="alert-message" style="margin-bottom: 0">{{{errorMessage}}}</div>{{/errorMessage}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does {{{errorMessage}}}
have triple (not double) curly braces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember right, I think that previously the error message string was using —
for the dash which required the 3 curly braces to display it correctly, but now it doesn't use it, so replaced them with 2.
I'm ready to look at this one, but there's a merge conflict. |
@redmunds Thanks for the review. I fixed the merge issues, updated the template and fixed the 3 curly braces issue. |
<label> | ||
{{PROJECT_SETTING_BASE_URL}}: <input type="text" placeholder="{{PROJECT_SETTING_BASE_URL_HINT}}" value="{{baseUrl}}" class="url" /> | ||
</label> | ||
{{#errorMessage}}<div class="alert" style="margin-bottom: 0">{{errorMessage}}</div>{{/errorMessage}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting technique for hiding elements in a Template!
Done with review. |
…rly braces to errorMessage
<label> | ||
{{Strings.PROJECT_SETTING_BASE_URL}}: <input type="text" placeholder="{{Strings.PROJECT_SETTING_BASE_URL_HINT}}" value="{{baseUrl}}" class="url" /> | ||
</label> | ||
{{#errorMessage}}<div class="alert" style="margin-bottom: 0">{{{errorMessage}}}</div>{{/errorMessage}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to re-add the third curly brace since the error string used is BASEURL_ERROR_INVALID_PROTOCOL
which uses $mdash;
. So I need to tell Mustache to parse this string as HTML and not just as plain text to display the dash correctly. If the string is modified, we can remove the third curly brace.
@redmunds I applied njx suggestion to use the strings as part of the template variables to fix the issue you mentioned, but had to add the removes third curly brace to the error message. |
I agree that is a better solution. I notice that there are other cases with template-based dialogs that use the same code that copies all member of the Strings Object that also need to be updated. Do you want to put up a separate pull request for that, or should I create a new Issue? |
Sure, I can open a new pull request to fix those other templates. |
@TomMalbran I opened #4252 to track the other templates. |
Thanks @TomMalbran. Merging. |
[OPEN] Move the Project Preferences Dialog variables to the template
This removes the jQuery dialog populate part of the Project Preferences Dialog, and moves it all to the template, passing the required variables.