Closed vanessuniq closed 1 year ago
It seems like somehow the style for the "open controls" list got changed - lots of space between elements:
New commits
replace
buttons in 'Find & Replace' on non edit modeA few notes/questions:
Good items:
A few notes/questions:
- Find and Replace: Sort order still doesn't seem to be working
- Icons: Ryan and I would vote for the original icons, and can they go back to being right-aligned?
- Open Controls list: spacing between rows still seems off
- Component Info: Can Title and Description be added back also?
Good items:
- The version info in the title (in edit mode) is good
- The "show more" and "show less" is good
- The find and replace search seems to be working
- The count of satisfied controls is good
Ok, Thanks, will use the old icons.
def find
find = params.require(:find)
component_id = params.require(:id)
rules = Component.find_by(id: component_id).rules
checks = Check.where(base_rule: rules).where('LOWER(content) like LOWER(?)', "%#{find}%")
descriptions = DisaRuleDescription.where(base_rule: rules).where('LOWER(vuln_discussion) like LOWER(?)',
"%#{find}%")
rules = rules.where('LOWER(title) like LOWER(?)', "%#{find}%").or(
rules.where('LOWER(fixtext) LIKE LOWER(?)', "%#{find}%").or(
rules.where('LOWER(vendor_comments) LIKE LOWER(?)', "%#{find}%").or(
rules.where(id: checks.pluck(:base_rule_id) | descriptions.pluck(:base_rule_id))
)
)
).order(:rule_id)
render json: rules
end
- Can you please add a pic of how the open controls list looks like on your screen? and also what you mean by icons going back to being right aligned? I need to find the right alignment for all screen size, for it to display correctly. I attached a pic of how the open controls and all controls list look like on my screen.
- For the component info, the title and description display if present. I can also change it to display them when empty.
There a pic of this in the first comment but i'll add another with the icons right aligned as they were previously. Even on your screenshot the open controls list spacing is different than all controls which wasn't the case before this PR.
The component info should just be displayed as it previously was even if empty. That helps us see that it is empty and not out of sight out of mind.
Here's what the new list looks like on my screen:
Versus the old way:
And for the Find/Replace sorting, I'll see if I can debug that, but here's an example:
That's the bottom of the list, but in that case the items returned in the order [1,4,6,11,3,6,12]
- Can you please add a pic of how the open controls list looks like on your screen? and also what you mean by icons going back to being right aligned? I need to find the right alignment for all screen size, for it to display correctly. I attached a pic of how the open controls and all controls list look like on my screen.
- For the component info, the title and description display if present. I can also change it to display them when empty.
There a pic of this in the first comment but i'll add another with the icons right aligned as they were previously. Even on your screenshot the open controls list spacing is different than all controls which wasn't the case before this PR.
![]()
The component info should just be displayed as it previously was even if empty. That helps us see that it is empty and not out of sight out of mind.
Ok, I misunderstood Darrick previous comment. I thought he wanted to space between the items to be reduced.
To clarify my screenshot is from the current version and how it's always looked. So the vertical spacing in this PR between open controls is more than it was previously.
To clarify my screenshot is from the current version and how it's always looked. So the vertical spacing in this PR between open controls is more than it was previously.
Thanks for clarifying, I was paying attention to the horizontal, not vertical spacing . New commit should fix it. I will address the sorting in Find & Replace
in another PR
I think this latest stuff looks great!
And Vanessa you were correct with the Title/Description fields - they show up if there:
And the Case-Sensitive Find works. I'll mess with the sorting/ordering of the Find/Replace window...
The data coming back from the find function in the controller is sorted by rule_id correctly, but when the javascript Object gets created with the key/value pairs, the key is the "id" of the rule, and that object gets sorted (automagically) by those keys. Maybe just by dumb luck the component I picked to test with does not match those up 1 to 1, meaning the ids of the rules aren't in the same order as the rule_ids. So in the FindAndReplace.vue file, at line 54, some sort of sort function needs to be added to sort based on rule_id instead of id. Here's what I came up with, but probably could be implemented elsewhere as well:
<div v-for="[id, find_result] in Object.entries(find_results).sort(function(a,b) { if (a[1].rule_id > b[1].rule_id) { return 1; } else if (a[1].rule_id < b[1].rule_id) { return -1; } return 0; })" :key="
${find_results_ver}-${id}`"
`
Here's the data set I was logging out:
ID: 63790 RULE_ID: 000010 ID: 63806 RULE_ID: 000001 ID: 63807 RULE_ID: 000011 ID: 63833 RULE_ID: 000012 ID: 63835 RULE_ID: 000006 ID: 63852 RULE_ID: 000004 ID: 63853 RULE_ID: 000003
The data coming back from the find function in the controller is sorted by rule_id correctly, but when the javascript Object gets created with the key/value pairs, the key is the "id" of the rule, and that object gets sorted (automagically) by those keys. Maybe just by dumb luck the component I picked to test with does not match those up 1 to 1, meaning the ids of the rules aren't in the same order as the rule_ids. So in the FindAndReplace.vue file, at line 54, some sort of sort function needs to be added to sort based on rule_id instead of id. Here's what I came up with, but probably could be implemented elsewhere as well:
<div v-for="[id, find_result] in Object.entries(find_results).sort(function(a,b) { if (a[1].rule_id > b[1].rule_id) { return 1; } else if (a[1].rule_id < b[1].rule_id) { return -1; } return 0; })" :key="
${find_results_ver}-${id}" >
Here's the data set I was logging out:
ID: 63790 RULE_ID: 000010 ID: 63806 RULE_ID: 000001 ID: 63807 RULE_ID: 000011 ID: 63833 RULE_ID: 000012 ID: 63835 RULE_ID: 000006 ID: 63852 RULE_ID: 000004 ID: 63853 RULE_ID: 000003
Nice, I'll see where it's best to add it.
New commits:
Let me know if we need to include more of the advance fields.
Fix #575
@freddyfeelgood @rlakey is there anything else to address on this PR?
@vanessuniq One question - depending on the answer it could be resolved in a future update...
When satisfied controls are not nested, but are locked, then the lock shows:
But when the nested switch is enabled, then the lock isn't shown:
I'm thinking the lock (and review?) icon(s) should show regardless of the nesting status. The "copy" icon could disappear when nested (as it's currently implemented), but I think the others should show.
All other changes look good though - I like the find/replace options a lot.
@vanessuniq One question - depending on the answer it could be resolved in a future update...
When satisfied controls are not nested, but are locked, then the lock shows:
But when the nested switch is enabled, then the lock isn't shown:
I'm thinking the lock (and review?) icon(s) should show regardless of the nesting status. The "copy" icon could disappear when nested (as it's currently implemented), but I think the others should show.
All other changes look good though - I like the find/replace options a lot.
New commit should fix this.
This PR covers the following:
Also satisfies
menuFix #570 Fix #571 Fix #574 Fix #576