Skip to content

Conversation

LucasLefevre
Copy link
Collaborator

@LucasLefevre LucasLefevre commented Aug 27, 2025

With this commit, we are simplifying the carousel.
Previously, the user could define a different title for each carousel tab.
Now, it's only a single title for all tabs.

We are still not sure where we are going with carousels, but it seems the
current direction is to use carousel to show the same data (hence one single
title) but from different perspectives (tabs)

Task: 5039276

@robodoo
Copy link
Collaborator

robodoo commented Aug 27, 2025

Pull request status dashboard

@LucasLefevre LucasLefevre force-pushed the master-single-carousel-title-lul branch 4 times, most recently from 47cbef0 to a7b01c4 Compare August 29, 2025 06:42
@LucasLefevre
Copy link
Collaborator Author

robodoo rebase-merge

@robodoo
Copy link
Collaborator

robodoo commented Aug 29, 2025

Merge method set to rebase and merge, using the PR as merge commit message.

@LucasLefevre LucasLefevre changed the title Master single carousel title lul [IMP] carousel: one title to rule them all Aug 29, 2025
This reverts commit d866f9d.

See next commit message and task.

Task: 5039276
Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋

This reverts commit 54dd3b8.

See next commit message and task.

Task: 5039276
@LucasLefevre LucasLefevre force-pushed the master-single-carousel-title-lul branch from a7b01c4 to 70e2d05 Compare September 1, 2025 10:26
Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicks, otherwise LGTM

]);
});

test("Can edit the carousel title for a carousel item", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Can edit the carousel title for a carousel item

<templates>
<t t-name="o-spreadsheet-CarouselPanel">
<div class="o-carousel-panel h-100 overflow-auto">
<Section title.translate="Carousel Title" class="'o-carousel-title'">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Section title.translate="Carousel Title" class="'o-carousel-title'">
<Section title.translate="Carousel Title" class="'o-carousel-title pt-0'">

probably

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(in "Carousel Sections", not "Carousel Title" )

With this commit, we are simplifying the carousel.
Previously, the user could define a different title for each carousel tab.
Now, it's only a single title for all tabs.

We are still not sure where we are going with carousels, but it seems the
current direction is to use carousel to show the same data (hence one single
title) but from different perspectives (tabs)

Task: 5039276
@hokolomopo hokolomopo force-pushed the master-single-carousel-title-lul branch from 70e2d05 to 4bda34a Compare September 4, 2025 06:44
@hokolomopo
Copy link
Contributor

@robodoo r+

fixed the nitpicks

robodoo pushed a commit that referenced this pull request Sep 4, 2025
This reverts commit d866f9d.

See next commit message and task.

Task: 5039276
Part-of: #7003
Signed-off-by: Adrien Minne (adrm) <[email protected]>
robodoo pushed a commit that referenced this pull request Sep 4, 2025
This reverts commit 54dd3b8.

See next commit message and task.

Task: 5039276
Part-of: #7003
Signed-off-by: Adrien Minne (adrm) <[email protected]>
robodoo pushed a commit that referenced this pull request Sep 4, 2025
With this commit, we are simplifying the carousel.
Previously, the user could define a different title for each carousel tab.
Now, it's only a single title for all tabs.

We are still not sure where we are going with carousels, but it seems the
current direction is to use carousel to show the same data (hence one single
title) but from different perspectives (tabs)

Task: 5039276
Part-of: #7003
Signed-off-by: Adrien Minne (adrm) <[email protected]>
robodoo added a commit that referenced this pull request Sep 4, 2025
With this commit, we are simplifying the carousel.
Previously, the user could define a different title for each carousel tab.
Now, it's only a single title for all tabs.

We are still not sure where we are going with carousels, but it seems the
current direction is to use carousel to show the same data (hence one single
title) but from different perspectives (tabs)

closes #7003

Task: 5039276
Signed-off-by: Adrien Minne (adrm) <[email protected]>
@robodoo robodoo added the 18.5 label Sep 4, 2025
@robodoo robodoo closed this Sep 4, 2025
@fw-bot fw-bot deleted the master-single-carousel-title-lul branch September 11, 2025 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants