related_items field implementation #441
Conversation
|
Editor view with some items: Editor view, item hovered: Editor view, no items: Modal view, first item selected, second item hovered: Modal view, item open in preview: |
|
@thecalcc I don't really see anything here specific to Belga in this PR (except for checking of the search provider with a static string/url). IMO, this is a core feature and we should do our best to keep it a core feature. In my Planning PR I have added an improvement where if you add |
| PanelContentBlock, | ||
| ContentDivider, | ||
| } from 'superdesk-ui-framework/react'; | ||
| import {getProjectedFieldsArticle, gettext} from 'superdesk-core/scripts/core/utils'; |
There was a problem hiding this comment.
Please do not import anything from superdesk-core, it was something that we used to do years ago before the SuperdeskAPI. Now that we have that API it is best to use that, if needing anything from core then we should add the required functionality to that API.
|
|
||
| render(): React.ReactNode { | ||
| const {closeModal} = this.props; | ||
| const allLanguages = [ |
There was a problem hiding this comment.
This should ideally come from the languages defined in the system, see vocabs selector for an example
| <Spacer v gap="0"> | ||
| <div | ||
| style={{ | ||
| padding: 12, |
There was a problem hiding this comment.
Constant numbers should not be used, instead we have helper classes for this (https://ui-framework.superdesk.org/#/components/spacing)
| <div | ||
| style={{ | ||
| padding: 12, | ||
| backgroundColor: 'white' |
There was a problem hiding this comment.
Does this work with a dark theme? An issue with using inline styles vs styles defined in the UI-Framework
| </div> | ||
| <ContentDivider margin='none' /> | ||
| <Spacer h gap="0" justifyContent='start' alignItems='start' noWrap> | ||
| <div style={{...{width: this.state.previewItem ? '65%' : '100%'}, ...{padding: 12}}}> |
There was a problem hiding this comment.
Surely the width here can be done better, rather than constant percentage values (@fritzSF)?
Again with the padding, please use the helper classes
| </div> | ||
| } | ||
| <div style={{height: '100%'}}> | ||
| <Panel background='light' open={this.state.previewItem != null} side='right' size="medium"> |
There was a problem hiding this comment.
Does specifying background='light' work with a dark theme?
| @@ -0,0 +1,131 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
We already have an ArchivePreviewComponent, which is used in the Assignment side panel. Could this be used here? They both provide the same functionality, so it's best to stick to DRY principle of re-using an existing component
| @@ -0,0 +1,188 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Same thing here, stick with the DRY principle and use ArticleItemConcise component from the SuperdeskAPI.
This way this one component is used in multiple places to render related articles, rather than duplication of the same functionality.
If we need changes done, such as to show language, then this should be done on the ArticleItemConcise component
| @@ -0,0 +1,196 @@ | |||
| import {IArticle} from 'superdesk-api'; | |||
|
|
|||
| export const fakeEditor: any = { | |||
There was a problem hiding this comment.
I'm assuming this is not needed for production, only in testing/development. If so, please remove this
There was a problem hiding this comment.
We do need it, because the items coming from Belga don't have an actual content profile, but the search_provider_proxy from our backend works with projections, and those are gotten through a content profile. In the future I guess we can rework this so we don't need to fake content profiles.
|
Moved to superdesk-planning (superdesk/superdesk-planning#1931) |
SDBELGA-768