Skip to content

FloatingActionButton: added the component to be used in smaller views #2347

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

Merged
merged 8 commits into from
Sep 3, 2023
Merged

FloatingActionButton: added the component to be used in smaller views #2347

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d492fda
floating action button for mobile
dewanshDT Aug 4, 2023
c3a72d5
Merge branch 'develop' of https://github.com/processing/p5.js-web-edi…
dewanshDT Aug 6, 2023
7cf588a
else
dewanshDT Aug 6, 2023
daa8bf5
button foreground fix
dewanshDT Aug 6, 2023
b8adaf8
button styled component
dewanshDT Aug 6, 2023
11f696d
Merge branch 'develop' into dewanshmobile/FAB
dewanshDT Aug 14, 2023
fc4abb7
Merge branch 'develop' into dewanshmobile/FAB
lindapaiste Aug 15, 2023
b5aae6b
Merge branch 'develop' of https://github.com/processing/p5.js-web-edi…
dewanshDT Sep 2, 2023
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
65 changes: 65 additions & 0 deletions client/modules/IDE/components/FloatingActionButton.jsx
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just want to note some of the ideas that we've tossed around regarding this element, potentially for future refactorings but not for right now:

  • We could export the styled part as a reusable FloatingActionButton UI component, and make this specific StartStopButton be an instance of that common FloatingActionButton. (This fits with best practices, but is kind of silly since there are no other floating action buttons anywhere else in the app.)
  • We could extract the existing StartStopButton from the desktop toolbar into its own component (see Break up Toolbar elements #2239). That component would need to take a className prop in order to be stylable with styled-components. This floating version for mobile could be a styled(StartStopButton), where we only need to specify the styles to override like the position and size. The positioning could also potentially be handled by a separate div which has the button as a child.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, the StartStopButton is a great idea and since we do not have any more floating action buttons anywhere else in the app but when I made it I thought it would be reusable and we should definitely export the styled part as a floating action button and the start-stop button should be passed in as a child element. let me know when we want to do it

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React from 'react';
import styled from 'styled-components';
import classNames from 'classnames';
import { useDispatch, useSelector } from 'react-redux';
import PropTypes from 'prop-types';
import PlayIcon from '../../../images/triangle-arrow-right.svg';
import StopIcon from '../../../images/stop.svg';
import { prop, remSize } from '../../../theme';
import { startSketch, stopSketch } from '../actions/ide';

const Button = styled.button`
position: fixed;
right: ${remSize(20)};
bottom: ${remSize(20)};
height: ${remSize(60)};
width: ${remSize(60)};
z-index: 3;
padding: 0;
${prop('Button.secondary.default')};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another variable that's not assigned to any property.

aspect-ratio: 1/1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This property has way more support than I thought! It's not really necessary since you've defined both a width and a height but it's not hurting anything so 🤷 .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah 😂😂

border-radius: 99999px;
display: flex;
justify-content: center;
align-items: center;
box-shadow: rgba(0, 0, 0, 0.24) 0px 3px 8px;
&.stop {
${prop('Button.primary.default')}
Copy link
Collaborator

Choose a reason for hiding this comment

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

we've got a color here with no property!

g {
fill: ${prop('Button.primary.default.foreground')};
}
}
> svg {
width: 35%;
height: 35%;
> g {
fill: ${prop('Button.primary.hover.foreground')};
}
}
`;

const FloatingActionButton = (props) => {
const isPlaying = useSelector((state) => state.ide.isPlaying);
const dispatch = useDispatch();

return (
<Button
className={classNames({ stop: isPlaying })}
style={{ paddingLeft: isPlaying ? 0 : remSize(5) }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't love this but I know it's here to fix a genuine problem so it's best to keep it for now so as not to fall deep into an SVG rabbit hole. We need this because the PlayIcon does not appear visually centered within its viewbox. IMO that's a problem with the icon and we may want to modify the icon to have whitespace on the side so that it looks centered.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah exactly that's what I thought, but let's keep it for now we'll remove it when we decide to replace the Icon with a better one.

onClick={() => {
if (!isPlaying) {
props.syncFileContent();
dispatch(startSketch());
} else dispatch(stopSketch());
}}
>
{isPlaying ? <StopIcon /> : <PlayIcon />}
</Button>
);
};

FloatingActionButton.propTypes = {
syncFileContent: PropTypes.func.isRequired
};

export default FloatingActionButton;