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.

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

Jump to bottom

Likhith/76897/mobiledialog migration to tsx #13

Merged
merged 5 commits into from
Nov 20, 2022
Merged

Likhith/76897/mobiledialog migration to tsx #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47b74d5
feat: :bug: resolved fialing testcase
likhith-deriv Nov 14, 2022
56c7ac7
feat: :recycle: incorporated review comments
likhith-deriv Nov 15, 2022
704e0b1
feat: :recycle: incorporated review comments
likhith-deriv Nov 16, 2022
ed7563f
feat: :recycle: incorporated review comments
likhith-deriv Nov 16, 2022
59768c2
feat: :recycle: incorporated review comments
likhith-deriv Nov 16, 2022
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
10 changes: 5 additions & 5 deletions packages/components/src/components/div100vh-container/div100vh-container.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import Div100vh from 'react-div-100vh';
type TDiv100vhContainerProps = {
id?: string;
height_offset?: string;
is_bypassed: boolean;
is_disabled: boolean;
is_bypassed?: boolean;
is_disabled?: boolean;
Comment on lines +20 to +21
Copy link

Choose a reason for hiding this comment

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

@likhith-deriv Should we give them a default value of false at lines 30 and 31? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

To be more explicit we can. Else since they are boolean, undefined becomes false

Copy link

Choose a reason for hiding this comment

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

undefined becomes false in JS world, In TS world it’s true | false | undefined 😁

max_height_offset?: string;
className?: string;
max_autoheight_offset?: string;
Expand All @@ -27,8 +27,8 @@ type TDiv100vhContainerProps = {
const Div100vhContainer = ({
children,
className,
is_bypassed,
is_disabled,
is_bypassed = false,
is_disabled = false,
id,
height_offset,
max_autoheight_offset,
Expand All @@ -38,7 +38,7 @@ const Div100vhContainer = ({
height: max_autoheight_offset ? '' : height_rule,
maxHeight: max_autoheight_offset ? `calc(100rvh - ${max_autoheight_offset})` : '',
};
if (is_bypassed) return children;
if (is_bypassed) return children as JSX.Element;
Copy link

Choose a reason for hiding this comment

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

Suggested change
if (is_bypassed) return children as JSX.Element;
if (is_bypassed) return children;

@likhith-deriv Why force casting here? I don’t have any errors 🤔

Copy link
Author

Choose a reason for hiding this comment

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

The component that uses Div100vhContainer gets a error stating that Div100vhContainer is not a JSX element. Hence I either typecast it or wrap children in Ract.Fragment.
@mariya said she faced issues when children were wrapped with fragment. Hence went with typecasting approach
Screen Shot 2022-11-16 at 12 15 11 PM

return (
<Div100vh id={id} className={className} style={is_disabled ? {} : height_style}>
{children}
Expand Down
2 changes: 1 addition & 1 deletion ...nts/src/components/mobile-dialog/index.js → ...nts/src/components/mobile-dialog/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MobileDialog from './mobile-dialog.jsx';
import MobileDialog from './mobile-dialog';
import './mobile-dialog.scss';

export default MobileDialog;
52 changes: 28 additions & 24 deletions ...omponents/mobile-dialog/mobile-dialog.jsx → ...omponents/mobile-dialog/mobile-dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,21 @@ import Text from '../text/text';
import Icon from '../icon/icon';
import Div100vhContainer from '../div100vh-container';

const MobileDialog = props => {
type TMobileDialog = {
content_height_offset?: string;
onClose: React.MouseEventHandler;
has_content_scroll?: boolean;
portal_element_id: string;
renderTitle?: () => string;
title?: string;
visible?: boolean;
wrapper_classname?: string;
header_classname?: string;
has_full_height?: boolean;
footer?: React.ReactNode;
};

const MobileDialog = (props: React.PropsWithChildren<TMobileDialog>) => {
const {
title,
visible,
Expand All @@ -20,25 +34,27 @@ const MobileDialog = props => {
header_classname,
} = props;

const footer_ref = React.useRef(false);
const footer_ref = React.useRef<HTMLDivElement>(null);
const [footer_height, setHeight] = React.useState(0);
React.useLayoutEffect(() => {
if (footer_ref.current && !footer_height) {
setHeight(footer_ref.current.offsetHeight);
}
}, [footer, footer_height]);

const portal_element = document.getElementById(portal_element_id);

const checkVisibility = () => {
if (props.visible) {
document.body.style.overflow = 'hidden';
document.getElementById(portal_element_id).style.overflow = 'hidden';
if (portal_element) portal_element.style.overflow = 'hidden';
} else {
document.body.style.overflow = null;
document.getElementById(portal_element_id).style.overflow = null;
document.body.style.overflow = 'unset';
if (portal_element) portal_element.style.overflow = 'unset';
}
};

const scrollToElement = (parent, el) => {
const scrollToElement = (parent: HTMLInputElement, el: HTMLInputElement) => {
const viewport_offset = el.getBoundingClientRect();
const hidden = viewport_offset.top + el.clientHeight + 20 > window.innerHeight;
if (hidden) {
Expand All @@ -48,10 +64,11 @@ const MobileDialog = props => {
};

// sometimes input is covered by virtual keyboard on mobile chrome, uc browser
const handleClick = e => {
const handleClick = (e: React.MouseEvent<HTMLInputElement>) => {
e.stopPropagation();
if (e.target.tagName === 'INPUT' && e.target.type === 'number') {
const scrollToTarget = scrollToElement(e.currentTarget, e.target);
const target = e.target as HTMLInputElement;
if (target.tagName === 'INPUT' && target.type === 'number') {
const scrollToTarget = () => scrollToElement(e.currentTarget, target);
window.addEventListener('resize', scrollToTarget, false);

// remove listener, resize is not fired on iOS safari
Expand All @@ -62,7 +79,7 @@ const MobileDialog = props => {
};

checkVisibility();
if (!document.getElementById(portal_element_id)) return null;
if (!portal_element) return null;
return ReactDOM.createPortal(
<CSSTransition
appear
Expand Down Expand Up @@ -119,21 +136,8 @@ const MobileDialog = props => {
</Div100vhContainer>
</div>
</CSSTransition>,
document.getElementById(portal_element_id)
portal_element
);
};

MobileDialog.propTypes = {
content_height_offset: PropTypes.string,
children: PropTypes.any,
onClose: PropTypes.func,
has_content_scroll: PropTypes.bool,
portal_element_id: PropTypes.string.isRequired,
renderTitle: PropTypes.func,
title: PropTypes.string,
visible: PropTypes.bool,
wrapper_classname: PropTypes.string,
header_classname: PropTypes.string,
};

export default MobileDialog;