Move the refresh icon to the 'Change view' toolbar, left aligned #1161
Conversation
assets/styles/navbar.scss
Outdated
| } | ||
|
|
||
| .navbar--list-controls:has(.navbar__inner--icon) { |
There was a problem hiding this comment.
See if you can handle this in the rendering code rather than with styles
|
@tomaskikutis this is resizeObserverComponent |
| @@ -26,6 +62,8 @@ AgendaListViewControls.propTypes = { | |||
| hideFeaturedToggle: PropTypes.bool, | |||
| featuredFilter: PropTypes.bool, | |||
| hasAgendaFeaturedItems: PropTypes.bool, | |||
| newItems: PropTypes.array, | |||
| fetchItems: PropTypes.func, | |||
There was a problem hiding this comment.
remove prop types, convert to interface
| ); | ||
| }; | ||
|
|
||
| const renderRefreshButton = () => { |
There was a problem hiding this comment.
What's the reason to make it a function and call twice with no arguments versus defining and reusing it as a value?
const refreshButton: React.ReactNode = (newItems || []).length
? (
<div className="navbar__inner navbar__inner--icon">
<NewItemsIcon
newItems={newItems}
refresh={fetchItems}
/>
</div>
)
: null;
};
| } | ||
| <ListViewOptions setView={setView} activeView={activeView} /> | ||
| function AgendaListViewControls({activeView, setView, hideFeaturedToggle, toggleFeaturedFilter, featuredFilter, hasAgendaFeaturedItems, newItems, fetchItems}: any) { | ||
| const Wrapper = ({children}: any) => { |
There was a problem hiding this comment.
Add a comment on what is the purpose of the wrapper. Since it's a pure function move it to top level.
| @@ -63,6 +103,8 @@ ListViewControls.propTypes = { | |||
| hideSearchAllVersions: PropTypes.bool, | |||
| searchAllVersions: PropTypes.bool, | |||
| toggleSearchAllVersions: PropTypes.func, | |||
| newItems: PropTypes.array, | |||
| fetchItems: PropTypes.func, | |||
There was a problem hiding this comment.
drop prop types, convert to interfaces
| @@ -565,6 +565,10 @@ export function isMobilePhone() { | |||
| return isTouchDevice() && screen.width < 768; | |||
| } | |||
|
|
|||
| export function isMobilePhoneScreen(dimensions: number) { | |||
There was a problem hiding this comment.
unify mobile screen size. is it 575 or 768?
| }; | ||
|
|
||
| return ( | ||
| <ResizeObserverComponent> |
There was a problem hiding this comment.
use WithScreenSizeObserver - it reports changes to screen size while ResizeObserverComponent reports changes to a DOM element's size.
NHUB-581
Purpose
Move the refresh icon to the 'Change view' toolbar, left aligned
What has changed
Position of refresh icon
Steps to test
Post article from superdesk
Checklist
lodash.getwith optional chaining for modified code segmentsResolves: #[issue-number]