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

Remove special handling of embedded images and links. #289

Merged
merged 11 commits into from
Aug 2, 2022
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,4 @@
# misc
.DS_Store
.vscode
*.sw[a-p]

coverage
*.log
4,000 changes: 4,000 additions & 0 deletions docs/index.js

Large diffs are not rendered by default.

7 changes: 7 additions & 0 deletions docs/index.js.map

Large diffs are not rendered by default.

29 changes: 16 additions & 13 deletions src/Components/CameraControl.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,27 +65,28 @@ function onLoad(location, cameraControls) {
*/
export function onHash(location, cameraControls) {
const encodedParams = getHashParams(location, CAMERA_PREFIX)
if (encodedParams == undefined) {
if (encodedParams === undefined) {
return
}
setCameraFromEncodedPosition(encodedParams, cameraControls)
setCameraFromParams(encodedParams, cameraControls)
}


/**
* Set the camera position
* @param {String} encodedPosition camera position
* @param {String} encodedParams camera position
* @param {Object} cameraControls obtained from the viewer
*/
export function setCameraFromEncodedPosition(encodedPosition, cameraControls) {
export function setCameraFromParams(encodedParams, cameraControls) {
// addCameraUrlParams is accessed from the issue card and it is undefined on the first render
if (!cameraControls) {
return
}
const coords = parseHashParams(encodedPosition)
const coords = parseHashParams(encodedParams)
if (coords) {
cameraControls.setPosition(coords[0], coords[1], coords[2], true)
if (coords.length == 6) {
const extendedCoordsSize = 6
if (coords.length === extendedCoordsSize) {
cameraControls.setTarget(coords[3], coords[4], coords[5], true)
}
}
Expand All @@ -96,7 +97,7 @@ export function setCameraFromEncodedPosition(encodedPosition, cameraControls) {
const floatPattern = '(-?\\d+(?:\\.\\d+)?)'
const coordPattern = `${floatPattern},${floatPattern},${floatPattern}`
const paramPattern = `${CAMERA_PREFIX}:${coordPattern}(?:,${coordPattern})?`
const regex = new RegExp(paramPattern)
const paramRegex = new RegExp(paramPattern)


// Exported for testing
Expand All @@ -105,12 +106,13 @@ const regex = new RegExp(paramPattern)
* @return {Object|undefined} The coordinates if present and valid else undefined
*/
export function parseHashParams(encodedParams) {
const match = encodedParams.match(regex)
const match = encodedParams.match(paramRegex)
const stof = (str) => {
const val = parseFloat(parseFloat(str).toFixed(2))
const floatDigits = 2
const val = parseFloat(parseFloat(str).toFixed(floatDigits))
if (isFinite(val)) {
const rounded = parseFloat(val.toFixed(0))
return rounded == val ? rounded : val
return rounded === val ? rounded : val
} else {
console.warn('Invalid coordinate: ', str)
}
Expand Down Expand Up @@ -151,12 +153,13 @@ export function addCameraUrlParams(cameraControls) {
return
}
const position = cameraControls.getPosition()
let camArr = roundCoord(...position, 2)
const floatDigits = 2
let camArr = roundCoord(...position, floatDigits)
const target = cameraControls.getTarget()
if (target.x == 0 && target.y == 0 && target.z == 0) {
if (target.x === 0 && target.y === 0 && target.z === 0) {
camArr = camArr.concat(0)
} else {
camArr = camArr.concat(roundCoord(...target, 2))
camArr = camArr.concat(roundCoord(...target, floatDigits))
}
addHashParams(window.location, CAMERA_PREFIX, camArr)
}
Expand Down
100 changes: 61 additions & 39 deletions src/Components/IssueCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@ import Paper from '@mui/material/Paper'
import {makeStyles} from '@mui/styles'
import useStore from '../store/useStore'
import {assertDefined} from '../utils/assert'
import {addHashParams} from '../utils/location'
import {addHashParams, getHashParamsFromHashStr} from '../utils/location'
import {isRunningLocally} from '../utils/network'
import {findUrls} from '../utils/strings'
import {TooltipIconButton} from './Buttons'
import {ISSUE_PREFIX} from './IssuesControl'
import {setCameraFromEncodedPosition, addCameraUrlParams, removeCameraUrlParams} from './CameraControl'
import {
CAMERA_PREFIX,
addCameraUrlParams,
setCameraFromParams,
parseHashParams,
removeCameraUrlParams,
} from './CameraControl'
import {useIsMobile} from './Hooks'
import SelectIcon from '../assets/2D_Icons/Select.svg'
import CameraIcon from '../assets/2D_Icons/Camera.svg'
Expand All @@ -23,9 +30,7 @@ import ShareIcon from '../assets/2D_Icons/Share.svg'
* @param {string} title issue title
* @param {string} avatarUrl user avatarUrl
* @param {string} body issue body
* @param {string} imageUrl issue image
* @param {string} date issue date
* @param {string} embeddedUrl full url attached to GH issue with camera position
* @param {number} numberOfComments number of replies to the issue - refered to as comments in GH
* @param {boolean} expandedImage governs the size of the image, small proportions on mobile to start
* @param {boolean} isComment Comments/replies are formated differently
Expand All @@ -38,15 +43,12 @@ export default function IssueCard({
title = 'Title',
avatarUrl = '',
body = '',
imageUrl = '',
date = '',
embeddedUrl = '',
numberOfComments = null,
expandedImage = true,
isComment = false,
}) {
assertDefined(id)
assertDefined(index)
assertDefined(body, id, index)
const [expandText, setExpandText] = useState(false)
const [expandImage, setExpandImage] = useState(expandedImage)
const selectedIssueId = useStore((state) => state.selectedIssueId)
Expand All @@ -57,26 +59,40 @@ export default function IssueCard({
const selected = selectedIssueId === id
const bodyWidthChars = 80
const textOverflow = body.length > bodyWidthChars
const embeddedCameraParams = findUrls(body)
.filter((url) => {
if (url.indexOf('#') === -1) {
return false
}
const encoded = getHashParamsFromHashStr(
url.substring(url.indexOf('#') + 1),
CAMERA_PREFIX)
return encoded && parseHashParams(encoded)
})
const firstCamera = embeddedCameraParams[0] // intentionally undefined if empty
const isMobile = useIsMobile()
const classes = useStyles({expandText: expandText, select: selected, expandImage: expandImage, isComment: isComment})


useEffect(() => {
if (isMobile) {
setExpandImage(false)
}
}, [isMobile])


useEffect(() => {
if (selected && embeddedUrl) {
setCameraFromEncodedPosition(embeddedUrl, cameraControls)
if (selected && firstCamera) {
setCameraFromParams(firstCamera, cameraControls)
}
}, [selected, embeddedUrl, cameraControls])
}, [selected, firstCamera, cameraControls])


/** Selecting a card move the notes to the replies/comments thread. */
function selectCard() {
setSelectedIssueIndex(index)
setSelectedIssueId(id)
if (embeddedUrl) {
setCameraFromEncodedPosition(embeddedUrl)
if (embeddedCameraParams) {
setCameraFromParams(firstCamera)
}
addHashParams(window.location, ISSUE_PREFIX, {id: id})
}
Expand All @@ -87,9 +103,9 @@ export default function IssueCard({
* the issue/comment.
*/
function showCameraView() {
setCameraFromEncodedPosition(embeddedUrl, cameraControls)
setCameraFromParams(firstCamera, cameraControls)
addCameraUrlParams(cameraControls)
if (!embeddedUrl) {
if (!embeddedCameraParams) {
removeCameraUrlParams()
}
}
Expand All @@ -107,6 +123,14 @@ export default function IssueCard({
}


const classes = useStyles({
expandText: expandText,
select: selected,
expandImage: expandImage,
isComment: isComment,
})


return (
<Paper
elevation={0}
Expand Down Expand Up @@ -143,13 +167,13 @@ export default function IssueCard({
}}
/>
}
{embeddedUrl || numberOfComments > 0 ?
{embeddedCameraParams || numberOfComments > 0 ?
<CardActions
selectCard={selectCard}
numberOfComments={numberOfComments}
embeddedUrl={embeddedUrl}
embeddedCameras={embeddedCameraParams}
selected={selected}
onClickNavigate={showCameraView}
onClickCamera={showCameraView}
onClickShare={shareIssue}
/> : null
}
Expand Down Expand Up @@ -206,28 +230,34 @@ const ShowMore = ({onClick, expandText}) => {
}


const CardActions = ({onClickNavigate, onClickShare, numberOfComments, selectCard, embeddedUrl, selected}) => {
const CardActions = ({
onClickCamera,
onClickShare,
numberOfComments,
selectCard,
embeddedCameras,
selected}) => {
const [shareIssue, setShareIssue] = useState(false)
const classes = useStyles({embeddedUrl: embeddedUrl, shareIssue: shareIssue})
const hasCameras = embeddedCameras.length > 0
const classes = useStyles({embeddedCameras: hasCameras})
return (
<div className={classes.actions}>
<div className={classes.rightGroup}>
{embeddedUrl ?
{hasCameras ?
<TooltipIconButton
disable={true}
disabled={hasCameras}
title='Show the camera view'
size='small'
placement='bottom'
onClick={onClickNavigate}
onClick={onClickCamera}
icon={
<CameraIcon
className={classes.buttonNavigate}
className={classes.buttonCamera}
style={{width: '24px', height: '24px'}}
/>}
/> : null}
{selected &&
<TooltipIconButton
disable={true}
title='Share'
size='small'
placement='bottom'
Expand Down Expand Up @@ -305,6 +335,9 @@ const useStyles = makeStyles((theme) => ({
color: 'green',
textDecoration: 'underline',
},
'& img': {
width: '100%',
},
},
showMore: {
cursor: 'pointer',
Expand Down Expand Up @@ -372,17 +405,6 @@ const useStyles = makeStyles((theme) => ({
cursor: 'pointer',
marginRight: '2px',
},
image: {
width: '96%',
borderRadius: '10px',
border: '1px solid #DCDCDC',
},
imageContainer: {
display: 'flex',
justifyContent: 'center',
alignItems: 'center',
paddingTop: '5px',
},
username: {
fontSize: '10px',
},
Expand All @@ -391,8 +413,8 @@ const useStyles = makeStyles((theme) => ({
height: '24px',
backgroundColor: theme.palette.custom.highLight,
},
buttonNavigate: {
backgroundColor: (props) => props.embeddedUrl ?
buttonCamera: {
backgroundColor: (props) => props.embeddedCameras ?
theme.palette.custom.highLight : theme.palette.custom.disable,
color: 'black',
},
Expand Down
2 changes: 1 addition & 1 deletion src/Components/IssueCard.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ test('Camera Position control', () => {
<IssueCard
id={id}
index={index}
embeddedUrl="http://localhost:8080/share/v/p/index.ifc#c:-141.9,72.88,21.66,-43.48,15.73,-4.34"
body="Test body [test link](http://localhost:8080/share/v/p/index.ifc#c:-141.9,72.88,21.66,-43.48,15.73,-4.34)"
/>
</ShareMock>)
const showCamera = rendered.getByTitle('Show the camera view')
Expand Down