New ui profile #664
New ui profile #664
Conversation
|
@rptrchv changed base branch to new-ui, now there's a conflict. |
| let obj = document.getElementById(`${e.id}`) | ||
| let src = obj.getAttribute("data-src1"); | ||
| e.setAttribute('src', src); | ||
| } |
There was a problem hiding this comment.
Please remove all commits which add changes to this file. All those commits are in new-ui-actions and there will be conflicts.
There was a problem hiding this comment.
I can't do it myself without full understanding of the problem and proposed solution. If you can't provide that - pls fix it yourself
| /* Firefox */ | ||
| input[type=number] { | ||
| -moz-appearance:textfield; | ||
| } |
There was a problem hiding this comment.
Does this disable spins for number inputs? Why?
There was a problem hiding this comment.
Yes, it disables spins. But this one is misplaced. We can remove it from there.
Does this disable spins for number inputs? Why?
Yes, it disables spins, but this one is misplaced, we can remove it.
| </div> | ||
|
|
||
| </div> | ||
| <!-- <style> |
| <th scope="row">IP</th> | ||
| <td>{{ object.deviceinfo.ipv4_address|default_if_none:'N/A' }}</td> | ||
| <td class="wott-table-label" scope="row">IP</td> | ||
| <td>{{ object.deviceinfo.ipv4_address|default:'N/A' }}</td> |
There was a problem hiding this comment.
AFAIK ipv4_address and fqdn may also be empty, for example if an interface was not assigned an IP address yet. This new code will treat them as "N/A". I think we shouldn't make such changes in a redesign branch without discussing them in detail.
There was a problem hiding this comment.
IMO "N/A" quite fits this case. @vpetersson WDYT?
There was a problem hiding this comment.
It's not ideal, but let's leave it as is for now and iterate later.
| <div class="tab wott-box-shadow"> | ||
| {% include 'device_info_tabs.html' with active="overview" %} | ||
| <div class="wott-table-content "> | ||
| <div class="wott-table-conten"> |
| $('#revoke-button').click(() => mixpanel.track("Revoke")); | ||
| }); | ||
| </script> | ||
| {% endif %} |
There was a problem hiding this comment.
please remove this change
There was a problem hiding this comment.
why? if it improves readability and fix wrong indentation?
There was a problem hiding this comment.
- it's not wrong
- you've indented the whole block => no readability changes
- there are zero useful changes here, yet they will remain in history which is already cluttered.
There was a problem hiding this comment.
Just ignore this here. I'll avoid such things in the future
| self.assertContains(response, 'telnetd</li>') | ||
| self.assertContains(response, 'fingerd</li>') | ||
| self.assertContains(response, '\n telnetd\n </li>') | ||
| self.assertContains(response, '\n fingerd\n </li>') |
There was a problem hiding this comment.
Either use assertInHTML or choose a shorter pattern to look for. Adding a whitespace in HTML template shouldn't break tests.
| if 'global_policy' in request.POST: | ||
| form = FirewallStateGlobalPolicyForm(request.POST, instance=firewallstate) | ||
| if form.is_valid(): | ||
| # TODO: check isn't it enough to do `form.save()` here. |
There was a problem hiding this comment.
Some day in the future. I don't have plans/tasks for this ATM
There was a problem hiding this comment.
I guess I read this sentence wrong. Does it say there isn't enough checks? If so, then ok
There was a problem hiding this comment.
Nope, I meant we may do redundant work there. But it's not urgent to check/fix that
rptrchv
force-pushed
the
new-ui-profile
branch
from
9ab69fc to
7939edb
Compare
fixed |
RA row not to brake buttons font CVE to Vulnerabilities Align buttons build with <a> tag
a-martynovich
force-pushed
the
new-ui-profile
branch
from
7939edb to
29f6285
Compare
No description provided.