-
Notifications
You must be signed in to change notification settings - Fork 52
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
21824 Used textareas for latest review comments to preserve formatting #688
Conversation
- added vue-auto-resize package (for textareas) - awaited async function in FilingHistoryList.vue - replaced paragraph with textarea in TodoList.vue - replaced paragraph with textarea in ContinuationIn.vue
@@ -50,6 +50,7 @@ | |||
"sbc-common-components": "3.0.13", | |||
"vue": "2.7.14", | |||
"vue-affix": "^0.5.2", | |||
"vue-auto-resize": "^1.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as in auth-web for the previous corrrespondence textareas.
// if needed, highlight a specific filing | ||
if (this.highlightId) { | ||
const index = this.getFilings.findIndex(f => (f.filingId === this.highlightId)) | ||
if (index >= 0) { // safety check | ||
this.toggleFilingHistoryItem(index) | ||
await this.toggleFilingHistoryItem(index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this is now correct, previously it was OK because onFilingsChange() runs in the background anyway, and there was no effect if toggleFilingHistoryItem() continued to run even after onFilingsChange() exited (which is what happens when you don't await an async method).
if (this.tempRegNumber) { | ||
// auto-expand bootstrap filing | ||
// assumes this the only filing in the Filing History list (which it should be) | ||
this.setPanel(0) | ||
await this.toggleFilingHistoryItem(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, we do want to wait for toggleFilingHistoryItem() to complete before we enable the textarea. This is a work-around for a bug in the textarea where it sometimes doesn't auto-resize (where its height=0 even when there is text content).
/gcbrun |
Temporary Url for review: https://business-filings-dev--pr-688-7zmow8rm.web.app SB says, try these: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! And I see the unit tests are also updated.
Quality Gate passedIssues Measures |
Temporary Url for review: https://business-filings-dev--pr-688-7zmow8rm.web.app |
Issue #: bcgov/entity#21824
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the business-filings-ui license (Apache 2.0).