-
Notifications
You must be signed in to change notification settings - Fork 37
TAN-5512 - Added title into the folder description #12419
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
TAN-5512 - Added title into the folder description #12419
Conversation
|
...es/commercial/content_builder/app/services/content_builder/craftjs/default_layout_service.rb
Show resolved
Hide resolved
| <Box data-testid="descriptionBuilderFolderPreviewContent"> | ||
| <Title color="tenantText" variant="h1"> | ||
| {localize(folderTitle)} | ||
| </Title> |
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.
@jamesspeake I'm not sure about making this an optional widget in the content builder 🤔 If they forget or choose not to add it, I think we end up with a page with bad accessibility/incorrect html hierarchy right? Since we won't have an h1 title?
Or maybe I've misunderstood how this works?
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.
It would indeed be possible to not have the project title in the page, but it felt odd laying out the rest of the page without having the title as part of the layout. Kind of why I added it as a separate PR as I was not totally sure. Good point about accessibility, but this is also possible on the homepage builder already.
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.
Good point about the homepage builder - in that case, I think this is fine then indeed 👍
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.
Left a couple comments, but not blocking (unless the accessibility issue is indeed possible) :)
(I didn't do any functional testing).
e0226fe
into
TAN-5512-content-builder-on-folders
Added folder title into editable area for folder