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

syslog-ng log viewer bootstrap conversion (Bug #7009) #278

Closed
wants to merge 5 commits into from

Conversation

@doktornotor
Copy link
Contributor

doktornotor commented Jan 31, 2017

Convert to bootstrap plus add some fixes and sanity checking while here.

Still no fancy stuff like Ajax, will need to wait for later.

@rbgarga rbgarga requested a review from sbeaver-netgate Jan 31, 2017
<div class="panel-heading"><h2 class="panel-title"><?=gettext("Syslog-ng Log Viewer Settings")?></h2></div>
<div class="panel-body table-responsive">
<table class="table table-condensed">
<tbody><tr><td>

This comment has been minimized.

Copy link
@sbeaver-netgate

sbeaver-netgate Jan 31, 2017

Contributor

This code should be converted to PHP Form classes for consistent appearance and to avoid nested tables. e.g.:

$section = new section("Syslog-ng Log Viewer Settings");

$section->addInput(Form_Select(
   'logfile',
   'Log File',
   ...

This comment has been minimized.

Copy link
@doktornotor

doktornotor Jan 31, 2017

Author Contributor

Well guys, this thing is getting way bigger PITA than it's worth. If someone else wants to take on it from here, feel free. I frankly have no clue how to convert this to form classes.

</table>
<tr><th width="22%">Include Archives</th><td width="78%"><input type="checkbox" name="archives" <?php if($archives) echo " CHECKED"; ?> /></td></tr>
<tr><th width="22%">Filter</th><td width="78%"><input name="filter" value="<?=$filter?>" /></td></tr>
<tr><th width="22%">Inverse Filter (NOT)</th><td width="78%"><input type="checkbox" name="not" <?php if($not) echo " CHECKED"; ?> /></td></tr>

This comment has been minimized.

Copy link
@sbeaver-netgate

sbeaver-netgate Jan 31, 2017

Contributor

Column widths should be handled automatically by the class="table-responsive" line above. If you really need to override that use <th class="col-sm-2"> (or other column specifier)

This comment has been minimized.

Copy link
@doktornotor

doktornotor Jan 31, 2017

Author Contributor

Well, letting that handled automatically produces awful useless spacing in the form.

@sbeaver-netgate

This comment has been minimized.

Copy link
Contributor

sbeaver-netgate commented Jan 31, 2017

I will look into the Bootstrap conversion. You can take any other changes from there.

@doktornotor

This comment has been minimized.

Copy link
Contributor Author

doktornotor commented Jan 31, 2017

Redid that table without hardcoded width; other than that, really not touching it. Still better to have it ugly and working than pretty and broken. :P

@sbeaver-netgate

This comment has been minimized.

Copy link
Contributor

sbeaver-netgate commented Jan 31, 2017

I have done the Bootstrap conversion on this file. Would you please test it, then apply your original improvements on the new version?

f095dbb

@doktornotor

This comment has been minimized.

Copy link
Contributor Author

doktornotor commented Jan 31, 2017

Closing due to conflicts. The seriously was NOT the smartest sequence of doing things.

@doktornotor doktornotor deleted the doktornotor:patch-9 branch Jan 31, 2017
doktornotor added a commit to doktornotor/FreeBSD-ports that referenced this pull request Jan 31, 2017
netgate-git-updates pushed a commit that referenced this pull request Feb 1, 2017
…ecking

Redo the rest of #278
(cherry picked from commit 008a392)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.