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

feat(Viewer): Add isPublic prop for manage panel blocks displayed #2563

Merged
merged 2 commits into from Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions react/Viewer/Footer/BottomSheetContent.jsx
Expand Up @@ -5,9 +5,9 @@ import { BottomSheetItem } from '../../BottomSheet'

import getPanelBlocks, { getPanelBlocksSpecs } from '../Panel/getPanelBlocks'

const BottomSheetContent = ({ file }) => {
const BottomSheetContent = ({ file, isPublic }) => {
const panelBlocks = getPanelBlocks({
panelBlocksSpecs: getPanelBlocksSpecs(),
panelBlocksSpecs: getPanelBlocksSpecs(isPublic),
file
})

Expand All @@ -24,7 +24,7 @@ const BottomSheetContent = ({ file }) => {

BottomSheetContent.propTypes = {
file: PropTypes.object.isRequired,
contactsFullname: PropTypes.string
isPublic: PropTypes.bool
}

export default BottomSheetContent
5 changes: 3 additions & 2 deletions react/Viewer/Footer/FooterContent.jsx
Expand Up @@ -23,7 +23,7 @@ const useStyles = makeStyles(theme => ({
}
}))

const FooterContent = ({ file, toolbarRef, children }) => {
const FooterContent = ({ file, toolbarRef, children, isPublic }) => {
const styles = useStyles()

const toolbarProps = useMemo(() => ({ ref: toolbarRef }), [toolbarRef])
Expand Down Expand Up @@ -54,7 +54,7 @@ const FooterContent = ({ file, toolbarRef, children }) => {
>
{FooterActionButtonsWithFile}
</BottomSheetHeader>
<BottomSheetContent file={file} />
<BottomSheetContent file={file} isPublic={isPublic} />
</BottomSheet>
)
}
Expand All @@ -68,6 +68,7 @@ const FooterContent = ({ file, toolbarRef, children }) => {
FooterContent.propTypes = {
file: PropTypes.object.isRequired,
toolbarRef: PropTypes.object,
isPublic: PropTypes.bool,
children: PropTypes.oneOfType([
PropTypes.node,
PropTypes.arrayOf(PropTypes.node)
Expand Down
7 changes: 4 additions & 3 deletions react/Viewer/Panel/PanelContent.jsx
Expand Up @@ -9,9 +9,9 @@ import { withViewerLocales } from '../hoc/withViewerLocales'

import getPanelBlocks, { getPanelBlocksSpecs } from './getPanelBlocks'

const PanelContent = ({ file, t }) => {
const PanelContent = ({ file, isPublic, t }) => {
const panelBlocks = getPanelBlocks({
panelBlocksSpecs: getPanelBlocksSpecs(),
panelBlocksSpecs: getPanelBlocksSpecs(isPublic),
file
})

Expand Down Expand Up @@ -43,7 +43,8 @@ const PanelContent = ({ file, t }) => {
}

PanelContent.propTypes = {
file: PropTypes.object.isRequired
file: PropTypes.object.isRequired,
isPublic: PropTypes.bool
}

export default withViewerLocales(PanelContent)
4 changes: 4 additions & 0 deletions react/Viewer/ViewerContainer.jsx
Expand Up @@ -26,6 +26,7 @@ const ViewerContainer = props => {
editPathByModelProps,
children,
componentsProps,
isPublic,
...rest
} = props
const { currentIndex, files, currentURL } = props
Expand Down Expand Up @@ -66,6 +67,7 @@ const ViewerContainer = props => {
/>
</EncryptedProvider>
<ViewerInformationsWrapper
isPublic={isPublic}
disableFooter={disableFooter}
validForPanel={validForPanel}
currentFile={currentFile}
Expand Down Expand Up @@ -107,6 +109,8 @@ ViewerContainer.propTypes = {
disablePanel: PropTypes.bool,
/** Show/Hide the panel containing more information about the file only on Phone & Tablet devices */
disableFooter: PropTypes.bool,
/** If the Viewer is in public view */
isPublic: PropTypes.bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sur le Viewer on avait commencé à ranger les props dans des ComponentsProps, est-ce qu'on ne pourrait pas mettre ce isPublic dedans ? 🤔 Quitte à faire un nouvel attribut "panel" dans ce ComponentsProps...

Pour l'instant isPublic ne sert que dans le Panel, si demain il sert aussi dans la toolbar, faudra le rajouter...

D'ailleurs ça me fait penser, est-ce qu'on ne devrait pas plutôt appeler ça "conditionXXX" ou quelque chose dans le genre afin de pouvoir rajouter des conditions propre à l'app dans les condition d'affichage des éléments du panel, le rendre moins "lié" à isPublic qui est un besoin de l'app Drive...

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: isPublic est quand même assez courant chez Cozy, c'est pas que du drive. C'est du partage et c'est cozy core.

Copy link
Member Author

Choose a reason for hiding this comment

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

En effet isPublic est à mon sens plus haut niveau, si d'aventure nous avons besoin de le propager ailleurs, nous aviserons mais passer par un Provider serait sans doute une meilleure approche.

Pour clarifier, dans l'exemple de componentsProps.OnlyOfficeViewer, la prop isEnabled est propre à OnlyOfficeViewer et n'a pas de sens d'être partagée à d'autres.
isPublic ne rentre pas dans ce cas, et nécessite une approche plus globale.

/* Props passed to components with the same name */
componentsProps: PropTypes.shape({
/** Used to open an Only Office file */
Expand Down
10 changes: 8 additions & 2 deletions react/Viewer/ViewerInformationsWrapper.jsx
Expand Up @@ -15,6 +15,7 @@ const ViewerInformationsWrapper = ({
disableFooter,
validForPanel,
toolbarRef,
isPublic,
children
}) => {
const theme = useTheme()
Expand All @@ -34,14 +35,18 @@ const ViewerInformationsWrapper = ({
<>
{!disableFooter && (
<Footer>
<FooterContent file={currentFile} toolbarRef={toolbarRef}>
<FooterContent
file={currentFile}
toolbarRef={toolbarRef}
isPublic={isPublic}
>
{children}
</FooterContent>
</Footer>
)}
{validForPanel && (
<InformationPanel>
<PanelContent file={currentFile} />
<PanelContent file={currentFile} isPublic={isPublic} />
</InformationPanel>
)}
</>
Expand All @@ -53,6 +58,7 @@ ViewerInformationsWrapper.propTypes = {
disableFooter: PropTypes.bool,
validForPanel: PropTypes.bool,
toolbarRef: PropTypes.object,
isPublic: PropTypes.bool,
children: PropTypes.oneOfType([
PropTypes.node,
PropTypes.arrayOf(PropTypes.node)
Expand Down