Skip to content

Infra/live code playground device wrapper #3298

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

Closed
wants to merge 45 commits into from

Conversation

adids1221
Copy link
Contributor

@adids1221 adids1221 commented Oct 7, 2024

Description

Docs playground (live-code) device wrapper to simulate Mobile or Tablet device.
In the buildDocs script wrapping the component snippet with the MobileDeviceWrapper that changes the size of the View container according to the selected mode.

Note: in buildDocs script there is a new function processSnippet that process the component snippet, since now we have cases that the live-code playground render a function or plain JSX that should be wrapped with the MobileDeviceWrapper.

Based on #3272

Changelog

Docs playground (live-code) device wrapper to simulate Mobile or Tablet device.

Additional info

MADS-4507

@adids1221 adids1221 added the docs docs related issue label Oct 7, 2024
@adids1221 adids1221 added this to the Docs milestone Oct 7, 2024
@adids1221 adids1221 requested a review from M-i-k-e-l October 7, 2024 16:57
@adids1221 adids1221 assigned M-i-k-e-l and unassigned adids1221 Oct 8, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove these changes of the icons 🙏

Comment on lines 49 to 57
const divTagIndex = processedSnippet.findIndex(line =>
line.trim().startsWith('return (') && processedSnippet[processedSnippet.indexOf(line) + 1]?.includes('<div>'));

if (divTagIndex !== -1) {
//If <div> tag is found, replace <div> and </div> with <MobileDeviceWrapper> and </MobileDeviceWrapper>
return processedSnippet
.map(line => line.replace('<div>', '<MobileDeviceWrapper>').replace('</div>', '</MobileDeviceWrapper>'))
.join('\n');
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code has a few assumptions, hopefully they don't break; we can try to fins a better solution with a convention we can set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's sync about it.
Iv'e tried to implement it as HOC but I had some issues.

@M-i-k-e-l M-i-k-e-l assigned adids1221 and unassigned M-i-k-e-l Oct 10, 2024
@adids1221 adids1221 requested a review from M-i-k-e-l October 13, 2024 11:21
@adids1221 adids1221 assigned M-i-k-e-l and unassigned adids1221 Oct 13, 2024
Copy link
Collaborator

@M-i-k-e-l M-i-k-e-l left a comment

Choose a reason for hiding this comment

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

This change breaks SortableGridList

[SimulatorTypes.TABLET]: {width: 900, height: 754}
};

const simulatorOptions = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be simulatorSegments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, changed.

style={styles.uilibLogo}
/>
<Text text60 marginV-s1>
Wix React Native UILIB
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Wix React Native UILIB
React Native UILIB

</View>
);

const findSelectedIndex = () => simulatorOptions.findIndex(option => option.value === type);
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about getSelectedIndex?

@@ -6069,7 +6069,7 @@ lodash.uniq@^4.5.0:
resolved "https://registry.npmjs.org/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773"
integrity sha512-xfBaXQd9ryd9dlSDvnvI0lvxfLJlYAZzXomUYzLKtUeOQvOP5piqAWuGtrhWeqaXK9hhoM/iyJc5AV+XfsX3HQ==

lodash@^4.17.20, lodash@^4.17.21:
lodash@4.x.x, lodash@^4.17.20, lodash@^4.17.21:
Copy link
Collaborator

Choose a reason for hiding this comment

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

yarn.lock should not change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right I'm not sure about that checking it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't managed to revert it, tried to checkout the commit and revert the file didn't managed to.

@M-i-k-e-l M-i-k-e-l assigned adids1221 and unassigned M-i-k-e-l Oct 14, 2024
@adids1221
Copy link
Contributor Author

@M-i-k-e-l Alexey made some changes in the Design.
Also I want to check the option of rendering an Iframe so we can use Picker and other components that extends Modal as in the mobile.

@adids1221
Copy link
Contributor Author

Closing this PR, #3351 has new approach with iframe.

@adids1221 adids1221 closed this Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs docs related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants