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.

Sign up for GitHub

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

Attempt at externalizing layout from extensions #829

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Attempt at externalizing layout from extensions #829

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8845ec5
Attempt at externalizing layout from extensions
May 3, 2021
98ada25
Allow unwrapped elements in ImageInfo
May 3, 2021
1fe3009
Oops, fixed sorting
May 3, 2021
e129714
Move some setup extension layout to theme.php
May 3, 2021
65640ee
Reduce layout functions in setup ext
May 4, 2021
836e84f
Standardized SetupBlock layout, moved to theme.php
May 4, 2021
4c6b405
Reimplemented avatar, improved ImageInfo layout code
May 4, 2021
b3df59d
format
May 4, 2021
69519a2
Fix empty rows on SetupBlocks
May 5, 2021
70ae508
setup theme using MicroHTML
May 5, 2021
221c135
Fixed bug with (pressumably) view counter extension
May 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/artists/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
global $user;
$artistName = $this->get_artistName_by_imageID($event->image->id);
if (!$user->is_anonymous()) {
$event->add_part($this->theme->get_author_editor_html($artistName), 42);
$event->add_part($this->theme->get_author_editor_html($artistName), 42, "Author");
}
}

Expand Down
9 changes: 2 additions & 7 deletions ext/artists/theme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,8 @@ public function get_author_editor_html(string $author): string
{
$h_author = html_escape($author);
return "
<tr>
<th>Author</th>
<td>
<span class='view'>$h_author</span>
<input class='edit' type='text' name='tag_edit__author' value='$h_author'>
</td>
</tr>
<span class='view'>$h_author</span>
<input class='edit' type='text' name='tag_edit__author' value='$h_author'>
";
}

Expand Down
5 changes: 1 addition & 4 deletions ext/image_view_counter/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
["image_id" => $event->image->id]
);

$event->add_part(
"<tr><th>Views:</th><td>$view_count</td></tr>",
38
);
$event->add_part($view_count, 38, "Views:");
}
}

Expand Down
2 changes: 1 addition & 1 deletion ext/post_titles/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
{
global $user;

$event->add_part($this->theme->get_title_set_html(self::get_title($event->image), $user->can(Permissions::EDIT_IMAGE_TITLE)), 10);
$event->add_part($this->theme->get_title_set_html(self::get_title($event->image), $user->can(Permissions::EDIT_IMAGE_TITLE)), 10, "Title");
}

public function onImageInfoSet(ImageInfoSetEvent $event)
Expand Down
11 changes: 2 additions & 9 deletions ext/post_titles/theme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@ class PostTitlesTheme extends Themelet
{
public function get_title_set_html(string $title, bool $can_set): string
{
$html = "
<tr>
<th>Title</th>
<td>
".($can_set ? "
$html = ($can_set ? "
<span class='view'>".html_escape($title)."</span>
<input class='edit' type='text' name='post_title' value='".html_escape($title)."' />
" : html_escape("
$title
"))."
</td>
</tr>
";
"));
return $html;
}
}
3 changes: 2 additions & 1 deletion ext/rating/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
$event->image->rating,
$user->can(Permissions::EDIT_IMAGE_RATING)
),
80
80,
"Rating"
);
}

Expand Down
11 changes: 2 additions & 9 deletions ext/rating/theme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,14 @@ class RatingsTheme extends Themelet
public function get_rater_html(int $image_id, string $rating, bool $can_rate): string
{
$human_rating = Ratings::rating_to_human($rating);
$html = "
<tr>
<th>Rating</th>
<td>
".($can_rate ? "
$html = ($can_rate ? "
<span class='view'>$human_rating</span>
<span class='edit'>
".$this->get_selection_rater_html([$rating])."
</span>
" : "
$human_rating
")."
</td>
</tr>
";
");
return $html;
}

Expand Down
2 changes: 1 addition & 1 deletion ext/relationships/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function onTagTermParse(TagTermParseEvent $event)

public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
{
$event->add_part($this->theme->get_parent_editor_html($event->image), 45);
$event->add_part($this->theme->get_parent_editor_html($event->image), 45, "Parent");
}

public function onImageDeletion(ImageDeletionEvent $event)
Expand Down
9 changes: 2 additions & 7 deletions ext/relationships/theme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,13 @@ public function get_parent_editor_html(Image $image): string
$h_parent_id = $image->parent_id;
$s_parent_id = $h_parent_id ?: "None";

$html = "<tr>\n".
" <th>Parent</th>\n".
" <td>\n".
(
$html = (
!$user->is_anonymous() ?
" <span class='view' style='overflow: hidden; white-space: nowrap;'>{$s_parent_id}</span>\n".
" <input class='edit' type='number' name='tag_edit__parent' type='number' value='{$h_parent_id}'>\n"
:
$s_parent_id
).
" <td>\n".
"</tr>\n";
);
return $html;
}

Expand Down
13 changes: 3 additions & 10 deletions ext/rule34/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,9 @@ public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
$image_link = $config->get_string(ImageConfig::ILINK);
$url0 = $event->image->parse_link_template($image_link, 0);
$url1 = $event->image->parse_link_template($image_link, 1);
$html = (string)TR(
TH("Links"),
TD(
A(["href"=>$url0], "File Only"),
" (",
A(["href"=>$url1], "Backup Server"),
")"
)
);
$event->add_part($html, 90);
$html = (string)A(["href"=>$url0], "File Only")." (".
A(["href"=>$url1], "Backup Server").")";
$event->add_part($html, 90, "Links");
}

public function onAdminBuilding(AdminBuildingEvent $event)
Expand Down
69 changes: 19 additions & 50 deletions ext/setup/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,17 @@ class SetupPanel
/** @var SetupBlock[] */
public array $blocks = [];
public BaseConfig $config;
public SetupTheme $theme;

public function __construct(BaseConfig $config)
public function __construct(BaseConfig $config, SetupTheme $theme)
{
$this->config = $config;
$this->theme = $theme;
}

public function create_new_block(string $title): SetupBlock
{
$block = new SetupBlock($title, $this->config);
$block = new SetupBlock($title, $this->config, $this->theme);
$this->blocks[] = $block;
return $block;
}
Expand All @@ -56,11 +58,13 @@ class SetupBlock extends Block
public ?string $header;
public ?string $body;
public BaseConfig $config;
public SetupTheme $theme;

public function __construct(string $title, BaseConfig $config)
public function __construct(string $title, BaseConfig $config, SetupTheme $theme)
{
parent::__construct($title, "", "main", 50);
$this->config = $config;
$this->theme = $theme;
}

public function add_label(string $text)
Expand Down Expand Up @@ -130,52 +134,14 @@ public function add_table_header_cell($content, int $colspan = 1)
$this->end_table_header_cell();
}

private function format_option(
string $name,
$html,
?string $label,
bool $table_row,
bool $label_row = false
) {
if ($table_row) {
$this->start_table_row();
}
if ($table_row) {
$this->start_table_header_cell($label_row ? 2 : 1, $label_row ? 'center' : 'right');
}
if (!is_null($label)) {
$this->body .= "<label for='{$name}'>{$label}</label>";
}

if ($table_row) {
$this->end_table_header_cell();
}

if ($table_row && $label_row) {
$this->end_table_row();
$this->start_table_row();
}

if ($table_row) {
$this->start_table_cell($label_row ? 2 : 1);
}
$this->body .= $html;
if ($table_row) {
$this->end_table_cell();
}
if ($table_row) {
$this->end_table_row();
}
}

public function add_text_option(string $name, string $label=null, bool $table_row = false)
{
$val = html_escape($this->config->get_string($name));

$html = "<input type='text' id='{$name}' name='_config_{$name}' value='{$val}'>\n";
$html .= "<input type='hidden' name='_type_{$name}' value='string'>\n";

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}

public function add_longtext_option(string $name, string $label=null, bool $table_row = false)
Expand All @@ -186,7 +152,7 @@ public function add_longtext_option(string $name, string $label=null, bool $tabl
$html = "<textarea rows='$rows' id='$name' name='_config_$name'>$val</textarea>\n";
$html .= "<input type='hidden' name='_type_$name' value='string'>\n";

$this->format_option($name, $html, $label, $table_row, true);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row, true);
}

public function add_bool_option(string $name, string $label=null, bool $table_row = false)
Expand All @@ -205,7 +171,7 @@ public function add_bool_option(string $name, string $label=null, bool $table_ro

$html .= "<input type='hidden' name='_type_$name' value='bool'>\n";

$this->format_option($name, $html, null, $table_row);
$this->body .= $this->theme->format_option($name, $html, null, $table_row);
}

// public function add_hidden_option($name, $label=null) {
Expand All @@ -221,7 +187,7 @@ public function add_int_option(string $name, string $label=null, bool $table_row
$html = "<input type='number' id='$name' name='_config_$name' value='$val' size='4' style='text-align: center;' step='1' />\n";
$html .= "<input type='hidden' name='_type_$name' value='int' />\n";

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}

public function add_shorthand_int_option(string $name, string $label=null, bool $table_row = false)
Expand All @@ -230,7 +196,7 @@ public function add_shorthand_int_option(string $name, string $label=null, bool
$html = "<input type='text' id='$name' name='_config_$name' value='$val' size='6' style='text-align: center;'>\n";
$html .= "<input type='hidden' name='_type_$name' value='int'>\n";

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}

public function add_choice_option(string $name, array $options, string $label=null, bool $table_row = false)
Expand All @@ -241,6 +207,9 @@ public function add_choice_option(string $name, array $options, string $label=nu
$current = $this->config->get_string($name);
}

// Not on this branch yet. Also requires build_selector to be public since we're accesing it from a block, not a theme.
// $html = $this->theme->build_selector("_config_".$name, array_flip($options), "id='".$name."'", false, [$current]);

$html = "<select id='$name' name='_config_$name'>";
foreach ($options as $optname => $optval) {
if ($optval == $current) {
Expand All @@ -253,7 +222,7 @@ public function add_choice_option(string $name, array $options, string $label=nu
$html .= "</select>";
$html .= "<input type='hidden' name='_type_$name' value='string'>\n";

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}

public function add_multichoice_option(string $name, array $options, string $label=null, bool $table_row = false)
Expand All @@ -273,7 +242,7 @@ public function add_multichoice_option(string $name, array $options, string $lab
$html .= "<input type='hidden' name='_type_$name' value='array'>\n";
$html .= "<!--<br><br><br><br>-->\n"; // setup page auto-layout counts <br> tags

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}

public function add_color_option(string $name, string $label=null, bool $table_row = false)
Expand All @@ -283,7 +252,7 @@ public function add_color_option(string $name, string $label=null, bool $table_r
$html = "<input type='color' id='{$name}' name='_config_{$name}' value='{$val}'>\n";
$html .= "<input type='hidden' name='_type_{$name}' value='string'>\n";

$this->format_option($name, $html, $label, $table_row);
$this->body .= $this->theme->format_option($name, $html, $label, $table_row);
}
}

Expand Down Expand Up @@ -316,7 +285,7 @@ public function onPageRequest(PageRequestEvent $event)
$this->theme->display_permission_denied();
} else {
if ($event->count_args() == 0) {
$panel = new SetupPanel($config);
$panel = new SetupPanel($config, $this->theme);
send_event(new SetupBuildingEvent($panel));
$this->theme->display_page($page, $panel);
} elseif ($event->get_arg(0) == "save" && $user->check_auth_token()) {
Expand Down
32 changes: 32 additions & 0 deletions ext/setup/theme.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,36 @@ protected function sb_to_html(SetupBlock $block): string
";
return $html;
}

public function format_option(string $name, $html, ?string $label, bool $table_row, bool $label_row = false): string
{
$output = "";
if ($table_row) {
$output .= "<tr><th colspan='".($label_row ? 2 : 1)."' style='text-align: ".($label_row ? 'center' : 'right')."'>";
}
if (!is_null($label)) {
$output .= "<label for='{$name}'>{$label}</label>";
}

if ($table_row) {
$output .= "</th>";
}

if ($table_row && $label_row) {
$output .= "</tr><tr>";
}

if ($table_row) {
$output .= "<td colspan='".($label_row ? 2 : 1)."'>";
}
$output .= $html;
if ($table_row) {
$output .= "</td>";
}
if ($table_row) {
$output .= "</tr>";
}

return $output;
}
Copy link
Contributor Author

@luanalatte luanalatte May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main.php still uses a lot of small non-overridable functions to generate tables (I think we should avoid that) but at least this one can be overridden now.

EDIT: Thankfully, those functions are rarely used outside of the setup extension itself.
start_table and end_table are the only ones that appear in multiple extensions, but even then they're mostly used once before the options and once after. We can remove most of them safely.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the current approach of adding lots of TD/TH/TR elements seems flexible but messy - ideally this'd be another place where the extension just returns a list of {setting_name, human_name, value, display_style} and the theme takes care of HTML, but I'm not sure how to do that without massively reducing flexibility...

(This isn't something that needs fixing right away, just something that I have in the back of my mind)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems flexible right until you don't want to use tables 😄
Also, I'm noticing extensions create their tables in very different ways, which explains why I had so much trouble getting everything to look consistent on my theme. There's even setupblocks that don't use tables at all and just populate the items in the div.

I know it's going to look a lot less flexible when I commit this. But giving extensions that much flexibility leads to inconsistencies...
I'm also trying to send everything through this format_option function, which I now renamed to format_item and takes care of labels and text as well, and I created other functions in theme.php to wrap them in the table elements (the ones one would override to get rid of tables entirely)

We can discard it if you think it's too much anyway. I'm adapting all extensions to the revised format now, I'll probably commit after that.

}
8 changes: 4 additions & 4 deletions ext/tag_edit/main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,10 @@ public function onAddAlias(AddAliasEvent $event)

public function onImageInfoBoxBuilding(ImageInfoBoxBuildingEvent $event)
{
$event->add_part($this->theme->get_user_editor_html($event->image), 39);
$event->add_part($this->theme->get_tag_editor_html($event->image), 40);
$event->add_part($this->theme->get_source_editor_html($event->image), 41);
$event->add_part($this->theme->get_lock_editor_html($event->image), 42);
$event->add_part($this->theme->get_user_editor_html($event->image), 39, "Uploader");
$event->add_part($this->theme->get_tag_editor_html($event->image), 40, "Tags");
$event->add_part($this->theme->get_source_editor_html($event->image), 41, "Source");
$event->add_part($this->theme->get_lock_editor_html($event->image), 42, "Locked");
}

public function onTagTermCheck(TagTermCheckEvent $event)
Expand Down
Loading