Skip to content

[3.0] Theme split (wave 4, part 18) — give the avatar picker one gallery list and no generated script - #9441

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/theme-avatar-picker
Open

[3.0] Theme split (wave 4, part 18) — give the avatar picker one gallery list and no generated script#9441
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/theme-avatar-picker

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

Part of the #7933 split, wave 4.
Profile area.

The gallery was two selects. You picked a directory in the first, and a second appeared
holding its files — filled in by JavaScript from a flat list of every avatar file on the
forum
, written into the page:

var files = ["' . implode('", "', Utils::$context['avatar_list']) . '"];
var avatar = document.getElementById("avatar");
var cat = document.getElementById("cat");
var selavatar = ""; var avatardir = ""; var file = …;
var size = avatar.alt.substr(3, 2) + " " + avatar.alt.substr(0, 2) + String.fromCharCode(117, 98, 116);

and a second block below generated a swap_avatar() with one case per choice, each case
generated again from which choices the forum allows — about 45 lines of PHP producing about
40 lines of JavaScript to show one <div> and hide three.

There is one select now, with the directories as <optgroup>s, so picking an avatar is
one action instead of two and no list of filenames goes into the page. Each panel carries
data-avatar-choice, and profile.js reads that rather than being told which panels exist.

Two behaviours the old script had are kept:

  • focusing a field inside a panel selects that panel's radio (the onfocus attributes)
  • switching to Gravatar clears an address left over from one of the other choices — the
    condition for that travels as a data-clear-email attribute instead of being compiled
    into a case
One fix that the single list needs

Profile::getAvatars() marks a file as checked with:

'checked' => $line == Utils::$context['member']['avatar']['server_pic'],

server_pic holds the path (Oxygen/cards.png), $line is the bare name, so a file
inside a directory never came back selected. It did not show before, because the nested
list was built in JavaScript and matched by name there. It compares against the path now.

Utils::$context['avatar_list'] goes with the script that was its only reader.

Checked

Panel switching, both directions, against release-3.0 — identical at every step:

radio panels shown
on load (none) none
Gallery server_stored
My own picture external
Upload upload
Gravatar gravatar
No avatar none

Gravatar clearing an http://… address on the way in: same both ways. Focusing the
external url field inside its visible panel selects the external radio: same both ways.
Gallery selection updates the preview — Oxygen/cards.png gives
http://localhost:8080/avatars/Oxygen/cards.png with alt="cards" — and there is no
second select left on the page. No console errors from this code.

What could not be exercised

Choosing a gallery avatar and saving it does not work on release-3.0 for reasons outside
this change: the page is a fatal error for any member who has one (fixed separately in
#9440), and even with that fixed the value does not persist, because
User::updateMemberData() cannot turn the bare filename back into a url. So the
pre-selection above is right by construction but cannot be demonstrated until that is
sorted — Avatar::$choice reports none for a prepackaged avatar today, whatever the
column says.

Issues References (Fixes|Related|Closes)

Part of #7933
Related #9440

Co-Authored-By: live627 john@jbrock.us

The gallery was two selects: pick a directory in the first, and a second
appeared holding its files, filled in from a flat list of every file the
forum has written into the page. Everything that drove it - the file
list, the directory, the currently selected file, the avatar directory
url, the preview element - was generated as globals, and a second block
below wrote a swap_avatar() with one case per choice, each of them
generated again from which choices the forum allows.

There is one select now, with the directories as optgroups, so picking an
avatar is one action instead of two and no list of filenames has to be
written into the page. Each panel says which choice it belongs to with
data-avatar-choice, and profile.js reads that rather than being told
which panels exist.

Two things the old script did are kept: focusing a field inside a panel
selects that panel's radio, and switching to Gravatar clears an address
that came from one of the other choices. The condition for the second one
travels as a data attribute instead of being compiled into a case.

getAvatars() marks a file as checked by comparing its name against the
member's avatar, which for a file inside a directory is the path and not
the name, so a nested avatar could never come back selected. It compares
against the path now, which is what the single list needs.

Utils::$context['avatar_list'] goes with the script that was its only
reader.

Signed-off-by: Mathias Alberts <mathiaspapealbert@hotmail.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
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