feat(Activity): Add Title field - #2332
Conversation
…sideURL, embedded stragglers
hirokiterashima
left a comment
hirokiterashima
left a comment
There was a problem hiding this comment.
The functionality to specify a title for activities seems to work. 👍 @breity- can you look into styling?
I think that this PR should be just about the activity title. Right now, this PR also adds a border around the OR activity and the background color chooser. These are nice additions, but I think we should handle them in a separate PR, to limit the scope of review.
Can you try to merge the conflicts with the develop branch? If you need help, let us know.
Also, I added some minor comments/suggestions inline to this PR.
| }); | ||
| this.applicationRef.attachView(this.componentRef.hostView); | ||
| setTimeout(() => hostElement.focus()); | ||
| setTimeout(() => hostElement.focus({ preventScroll: true })); |
There was a problem hiding this comment.
Was scrolling to focused element preventing an issue before?
There was a problem hiding this comment.
When changing the attributes for an element in the authoring tool, the page would begin at the prompt entry, meaning you had to scroll up before seeing the entry for the activity title. This behavior is certainly not an old flaw in the code, but this was a fix that worked for me when it appeared.
| styleUrl: 'html-student.component.scss', | ||
| templateUrl: 'html-student.component.html' | ||
| templateUrl: 'html-student.component.html', | ||
| imports: [ComponentHeaderComponent] |
There was a problem hiding this comment.
alphabetize: move to top.
| selector: 'outside-url-student', | ||
| templateUrl: 'outside-url-student.component.html' | ||
| templateUrl: 'outside-url-student.component.html', | ||
| imports: [ComponentHeaderComponent] |
There was a problem hiding this comment.
alphabetize: move to top.
| <div | ||
| class="pt-4 px-6 pb-6 rounded-lg border-2 border-black" | ||
| [style.background-color]="componentContent.backgroundColor ? (componentContent.backgroundColor.startsWith('#') ? componentContent.backgroundColor : '#' + componentContent.backgroundColor) : 'transparent'" | ||
| > |
There was a problem hiding this comment.
This new outer div would cause a indentation of the entire contents of this file. @breity is this the best way to do this, or is there another way to achieve the same outcome?
There was a problem hiding this comment.
This should be removed from the PR anyways, as we'll address design changes to components in another PR.
| componentChanged(): void { | ||
| this.projectService.nodeChanged(); | ||
| this.projectService.saveProject(); | ||
| } |
There was a problem hiding this comment.
Is this necessary? AbstractComponentAuthoring.componentChanged already calls this.projectService.nodeChanged(), which in turn saves the project.
There was a problem hiding this comment.
This is a fragment from the saving issue for tiles in the authoring tool that we discussed last week, and that Jon said he wanted to look into. These changes will be removed completely with the removal of the OR style changes regardless.
hirokiterashima
changed the title
…g/edit-component-dialog.component.ts Co-authored-by: Hiroki Terashima <honchikun@gmail.com>
|
Coverage Impact Unable to calculate total coverage change because base branch coverage was not found. Modified Files with Diff Coverage (6) 🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
There was a problem hiding this comment.
LGTM. I updated some styles and added a check to only show the title element if it is set. I also removed support for HTML in the title, since titles should generally just be strings and then we don't have to add the sanitizer dependency.
I'm wondering if we should set the title by default when a new component is created or if we should leave it blank. I like the idea of each activity having a title, but I wonder if it will be annoying/frustrating for authors editing existing units that don't use activity titles. Something we can discuss more as we work on the VLE redesign.
breity
added
the
enhancement
3 participants
Changes
-Added an "Activity Title" field that is consistent across all component types which is threaded through the schema via ComponentComponent.title, the authoring dialog, and the student-facing display via component-header. Important to note this is not the SLE, but simply the student view for a teacher working in the authoring tool.
-The field is now a standalone angular component named EditComponentHeaderComponent which owns its own save methods instead of living inline in the edit dialog.
-The Activity Title on new components defaults to the name of the component for all components. We discussed changing this for activity types like "custom," but those decisions and changes are yet to be implemented.
-Fixed what I believe to be a pre-existing bug where the prompt on many activity types, such as the two comment-based activity types and label/graph/custom, where the prompt field wouldn't render until some other field was touched, forcing an update. This was changed by changing component.component.prompt into a getter.
-The open response changes from three weeks ago still exist but these are purely visual changes, and I am happy to remove them. These include the outline, the drop shadow on the box, the drop shadow on the buttons, and the interactivity with the text box.
Test
-Create a new component of any type and ensure the Activity Title prefills with the type name.
-Edit the Activity Title field and ensure that the changes update immediately both in the authoring tool and on the student preview.
-Check that the prompt renders immediately instead of needing an explicit update (without changing any other field).
-There is a merge conflict as develop has been changed while I've been working. Lmk if you want me to handle this locally.
Closes #