Skip to content
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

mobile and desktop invisible elements #3727

Draft
wants to merge 1 commit into
base: master-mysterious-egg
Choose a base branch
from

Conversation

loco-odoo
Copy link

No description provided.

@robodoo
Copy link

robodoo commented Dec 12, 2024

This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged.

@FrancoisGe FrancoisGe force-pushed the master-mysterious-egg branch from 467dadd to 5cab1c7 Compare December 12, 2024 15:09
@loco-odoo loco-odoo force-pushed the master-mysterious-egg-loco-2 branch 2 times, most recently from 9d42d96 to 213385b Compare December 12, 2024 15:48
@FrancoisGe FrancoisGe force-pushed the master-mysterious-egg branch from 18a0e98 to 58143fd Compare December 12, 2024 18:49
@loco-odoo loco-odoo force-pushed the master-mysterious-egg-loco-2 branch 5 times, most recently from 3faebbb to e0486d5 Compare December 13, 2024 10:23
@loco-odoo loco-odoo force-pushed the master-mysterious-egg-loco-2 branch from e0486d5 to fb721c4 Compare December 13, 2024 10:28
@@ -62,7 +63,10 @@ export class SnippetsMenu extends Component {
canRedo: false,
activeTab: this.props.isTranslation ? "customize" : "blocks",
currentOptionsContainers: undefined,
});
this.invisibleElsContext = reactive({

Choose a reason for hiding this comment

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

Why not do a useState ?

invisibleEls: [],
invisibleSelector: this.invisibleSelector,

Choose a reason for hiding this comment

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

It s ok to compute the value invisibleSelector just at the setup ?
What happens if a value in invisibleSelector change

useSubEnv({
mobileContext: this.state.mobileContext,

Choose a reason for hiding this comment

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

Maybe overkill to pass it in the env ? Maybe wait to have a real use case ? Props is enough in this case no ?

let nbrCall = 0;
patchWithCleanup(InvisibleElementsPanel.prototype, {
getInvisibleElementsEntries() {
nbrCall++;

Choose a reason for hiding this comment

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

In this type of use case, you can use expect.step :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants