Skip to content

[FIX] Composer: Fix assistant position #7288

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: saas-18.4
Choose a base branch
from
Open

[FIX] Composer: Fix assistant position #7288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/composer/composer/composer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ export class Composer extends Component<CellComposerProps, SpreadsheetChildEnv>
assistantStyle["max-height"] = `${availableSpaceAbove - CLOSE_ICON_RADIUS}px`;
// render top
// We compensate 2 px of margin on the assistant style + 1px for design reasons
assistantStyle.transform = `translate(0, calc(-100% - ${cellHeight + 3}px))`;
assistantStyle.top = `-3px`;
assistantStyle.transform = `translate(0, -100%)`;
Comment on lines +237 to +238
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: there is a space between the composer and the assistant. Maybe it's good enough 🤷
image

}
if (cellX + ASSISTANT_WIDTH > this.props.delimitation.width) {
// render left
Expand Down
4 changes: 2 additions & 2 deletions src/components/small_bottom_bar/small_bottom_bar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { CellComposerStore } from "../composer/composer/cell_composer_store";
import { CellComposerProps, Composer } from "../composer/composer/composer";
import { ComposerFocusStore, ComposerInterface } from "../composer/composer_focus_store";
import { css, cssPropertiesToCss } from "../helpers";
import { getBoundingRectAsPOJO, isIOS } from "../helpers/dom_helpers";
import { getBoundingRectAsPOJO } from "../helpers/dom_helpers";
import { RibbonMenu } from "./ribbon_menu/ribbon_menu";

interface Props {
Expand Down Expand Up @@ -105,7 +105,7 @@ export class SmallBottomBar extends Component<Props, SpreadsheetChildEnv> {
height: this.focus === "inactive" ? "26px" : "fit-content",
"max-height": `130px`,
}),
showAssistant: !isIOS(), // Hide assistant on iOS as it breaks visually
showAssistant: false, // Hide assistant in small composer as it gets cropped ATM
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a task to fix this, am I blind or we do we not care about this ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its on the task description - it's not properly visible due to the html structure because we have several overflow:hidden that we use for other purpose. since it's not usable, its beter to deactivate it and try to reintroduce it as a banner for bottom bar specifically

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I see why you disabled it, but I didn't find a task to fix it properly in the future

placeholder: this.composerStore.placeholder,
};
}
Expand Down
5 changes: 3 additions & 2 deletions tests/composer/autocomplete_dropdown_component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,9 @@ describe("composer Assistant", () => {
expect(assistantEL).toMatchSnapshot();
expect(assistantEL.style.width).toBe("300px");
const containerEL = fixture.querySelector<HTMLElement>(".o-composer-assistant-container")!;
const marginsOffset = rect.height + 3; // 3px for the border and margin
expect(containerEL.style.transform).toBe(`translate(0, calc(-100% - ${marginsOffset}px))`);

expect(containerEL.style.top).toBe(`-3px`); // 3px for the border and margin
expect(containerEL.style.transform).toBe(`translate(0, -100%)`);
});

test("composer assistant min-width is the same as the underlying cell", async () => {
Expand Down