[3.0] Theme split (wave 4, part 15) — move the issue-warning page's script into profile.js - #9436
Open
albertlast wants to merge 1 commit into
Open
Conversation
The page generated all of its JavaScript from the template, which meant one if branch per notification template, each carrying a whole message body through Utils::escapeJavaScript(). The bodies and the level texts go out as JSON now and profile.js does the rest, wiring itself up on DOMContentLoaded instead of through onclick and onchange attributes. Two things changed shape along the way: The current level is an <output> rather than a span, so the script can reach it as this.form.cur_level and does not have to know the id. It keeps percent_format, which the string comes from, in a data attribute next to it. The Preview button is disabled rather than hidden while the notification is switched off. .button sets display, and an author rule beats the [hidden] rule the browser brings, so hiding it needs an inline style; disabled says the same thing and picks up the styling that is already there for it. The "create a template" link next to it is a plain span, so that one does hide. Signed-off-by: Mathias Alberts <mathiaspapealbert@hotmail.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part of the #7933 split, wave 4.
Profile area, first slice.
template_issueWarning()generated all of its JavaScript.populateNotifyTemplate()waswritten out as one
ifbranch per notification template, each carrying a whole messagebody through
Utils::escapeJavaScript();updateSlider()the same, one branch per warninglevel. The handlers were attached with
onclick/onchangeattributes, and a second<script>block at the bottom of the page held the jQuery preview call.The template now emits only the data:
and
profile.js— which every profile page already loads — carriesmodifyWarnNotify(),populateNotifyTemplate(),updateSlider()andajax_getTemplatePreview(), wiring itselfup on
DOMContentLoaded. The preview call isfetchrather than$.ajax.Two things changed shape
The current level is an
<output name="cur_level" for="warning_level">instead of<span id="cur_level_div">, so the script reaches it asthis.form.cur_leveland does notneed to know an id.
percent_format— which the text is built from, and which the oldinline script also used — rides along in a
data-formatattribute next to it, so thestring stays localised.
The Preview button is
disabledwhile the notification is switched off, where it usedto be hidden.
.buttonsetsdisplay: inline-block, and an author rule beats the[hidden]rule the browser brings, sohiddendoes nothing to it — hiding it needs aninline style.
disabledsays the same thing and picks up the stylingindex.css:175already has for it. The "create a template" link beside it is a plain
<span>, so that onestill hides.
Checked
Same page, before and after, on the slider (
inputnow, so it tracks the drag rather thanwaiting for release) — identical text at every stop:
0% (None.)/5% (None.)10% (User will be added to moderator watch list.)35% (All users posts will be moderated.)60% / 95% / 100% (User will not be able to post.)One thing that had to be got right: both operands are strings, so a plain
>=comparesthem as text and
"100"sorts below"35".Number()on each side.Also checked each of the three templates fills the body with the same text as before,
that picking the placeholder row leaves the body alone, that the notify checkbox enables
and disables the same four controls in both directions, and the preview both ways — a
good body renders into
#box_preview, an empty one puts "You selected to notify the userbut did not fill in the subject/message fields" into
#profile_error. No console errors,nothing in
smf_log_errors.The
is_mebranch draws the slider with no notification block at all, and the scriptguards for that separately. It is not reachable through the menu as things stand
(
Main.phpgives the area'own' => []), so it was exercised by granting thattemporarily.
Issues References (Fixes|Related|Closes)
Part of #7933
Co-Authored-By: live627 john@jbrock.us