Standardise NodeProcessor.data.ts constant names #2483
Conversation
| export const PROCESS_PANEL_HEADER_NO_OVERRIDE = ` | ||
| <panel header="# Lorem ipsum" alt="**strong alt**"> | ||
| <div slot="header"> | ||
| This existing header slot should be preserved in favour over header attribute. | ||
| </div> | ||
| Header attribute should not be inserted under panel since there is both an alt attribute and header slot, | ||
| but should be deleted. | ||
| Alt attribute should be inserted under panel as slot. | ||
| </panel> | ||
| `; | ||
|
|
||
| export const PROCESS_PANEL_HEADER_NO_OVERRIDE_EXPECTED = ` | ||
| <panel><template #_alt><p><strong>strong alt</strong></p> | ||
| </template> | ||
| <template #header><div> | ||
| This existing header slot should be preserved in favour over header attribute. | ||
| </div></template> | ||
| Header attribute should not be inserted under panel since there is both an alt attribute and header slot, | ||
| but should be deleted. | ||
| Alt attribute should be inserted under panel as slot. | ||
| </panel> | ||
| `; |
There was a problem hiding this comment.
Deleted this constant instead of the other one because this also tests for whether the alt attribute is processed correctly, which is tested for in the POST_PROCESS_PANEL_ID_ASSIGNED_USING_HEADER_SLOT test in lines 66 to 76
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2483 +/- ##
==========================================
+ Coverage 51.11% 51.14% +0.03%
==========================================
Files 124 124
Lines 5355 5359 +4
Branches 1152 1152
==========================================
+ Hits 2737 2741 +4
Misses 2328 2328
Partials 290 290 ☔ View full report in Codecov by Sentry. |
| Alt attribute should be inserted under panel as slot. | ||
| </panel> | ||
| `; | ||
|
|
||
| // Post Process | ||
|
|
||
| export const POST_PROCESS_PANEL_ID_ASSIGNED_USING_HEADER_SLOT = ` |
There was a problem hiding this comment.
Let me know if I'm missing something here, but it doesn't look like this one has an alt slot for it to test for it being inserted under panel as stated in your earlier comment?
There was a problem hiding this comment.
:O You are right - I think I got confused by the text in the constant. Will delete the other constant then
| export const PROCESS_QUESTION_ATTRIBUTES_NO_OVERRIDE = ` | ||
| <question header="**header**" hint="**hint**" answer="**answer**"> | ||
| export const PROCESS_QUESTION_HEADER_SLOT_TAKES_PRIORITY = ` | ||
| <question header="**header**"> |
There was a problem hiding this comment.
NIT: Since this text is overridden - maybe you can change the overridden text to "lorum ipsum" or something like in the other tests? This was confusing to me because I didn't realise the difference between what was overridden for a while.
Similar for hint & answer below
There was a problem hiding this comment.
LGTM! Thank you for the excellent work as usual @luminousleek
|
@kaixin-hc Each PR must have a SEMVER impact label, please remember to label the PR properly. |
What is the purpose of this pull request?
Partially addresses #2476
Overview of changes:
PROCESS_[ELEMENT]_[ATTRIBUTE]_SLOT_TAKES_PRIORITYformatnpm run test -- NodeProcessor.test.tshas been run and all tests pass.Anything you'd like to highlight/discuss:
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Standardise NodeProcessor constant names
Currently, there are two naming formats for constant names. This
has led to a redundant test being created in a previous PR.
Let's standardise the naming format to make the purpose of these
constants clearer. Let's also split some constants to test for one
overridden attribute at a time.
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):