Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| foreach (var skill in HboIDomain.ProfessionalSkills) | ||
| { | ||
| var value = skills.First(x => x.skillId == skill.Id); | ||
| var cell = CreateTableCellWithBorders("500"); | ||
| cell.FirstChild?.Append(new Shading | ||
| { Fill = value.maxMastery.Color, }); | ||
| cell.Append(new Paragraph(new Run(new Text($"{value.count}")))); | ||
| valueRow.AppendChild(cell); | ||
| } |
Check notice
Code scanning / CodeQL
Missed opportunity to use Select
There was a problem hiding this comment.
This is indeed something that can be used
NealGeilen
left a comment
NealGeilen
left a comment
There was a problem hiding this comment.
There are some visual changes that need to change.
- The KPI level legend needs to come first. Not after the tables.
- The tables are difficult to read because it's a bit cramped and words are cut-off. Personally, I would go for the original style of the component
There is enough space to show everything.
|
Could you povide a screenshot of the Word output? |
| body.Append(new Paragraph(new Run(new Text("")))); | ||
| body.AppendChild(GetTasks()); | ||
| body.Append(new Paragraph(new Run(new Text("")))); | ||
| body.AppendChild(GetSkills()); | ||
| body.Append(new Paragraph(new Run(new Text("")))); | ||
| body.AppendChild(GetLegend()); | ||
| body.Append(new Paragraph(new Run(new Text("")))); |
There was a problem hiding this comment.
Are these body.Append(new Paragraph(new Run(new Text("")))); nessecary. It seems kind of bloated.
There was a problem hiding this comment.
If these are not included, all the tables will merge together, will be automatically fixed with the refactor
There was a problem hiding this comment.
Can you atleast replace it with something like a line break?
| var headerRow = new TableRow(); | ||
|
|
||
| // Empty top-left cell. | ||
| headerRow.AppendChild(CreateTableCellWithBorders("2500", new Paragraph(new Run(new Text(""))))); |
There was a problem hiding this comment.
Could you specify what these values are "2500" (maybe make a constant)? Same here with the paragraph stuff.
There was a problem hiding this comment.
idk what these values are, still need to figure out what they do
| // Set table properties for formatting. | ||
| table.AppendChild(new TableProperties( | ||
| new TableWidth { | ||
| Width = "4", Type = TableWidthUnitValues.Auto, })); |
There was a problem hiding this comment.
Expand these to make the code look nices
Typiqally
changed the title
Current word output, needs to be changes to Neal's suggestion but that will take a large refactor of this code
|
|
The level legend should be under the tables and the "mastered, insufficient, etc." legend can be removed as it is not part of this component. Unless you added it to your screenshot on accident? |
3 participants
I'm pretty much done here, ill keep implementing more stuff unless someone stops me but yeah