Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Commit

Permalink
Merge pull request #371 from guardian/oliver/remove-onheightchange
Browse files Browse the repository at this point in the history
Removed onHeightChange prop
  • Loading branch information
oliverlloyd authored Jan 4, 2021
2 parents a27dbdb + 8b2ea3a commit c40232d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 31 deletions.
8 changes: 0 additions & 8 deletions src/App.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export const LoggedOutHiddenPicks = () => (
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand Down Expand Up @@ -68,7 +67,6 @@ export const InitialPage = () => (
expanded={true}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand Down Expand Up @@ -96,7 +94,6 @@ export const Overrides = () => (
expanded={true}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand All @@ -122,7 +119,6 @@ export const LoggedInHiddenNoPicks = () => (
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand All @@ -149,7 +145,6 @@ export const LoggedOutHiddenNoPicks = () => (
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand Down Expand Up @@ -177,7 +172,6 @@ export const Closed = () => (
expanded={true}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand All @@ -202,7 +196,6 @@ export const NoComments = () => (
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand All @@ -229,7 +222,6 @@ export const LegacyDiscussion = () => (
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>
</div>
);
Expand Down
7 changes: 1 addition & 6 deletions src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ describe('App', () => {
}}
apiKey="discussion-rendering-test"
onPermalinkClick={() => {}}
onHeightChange={() => {}}
/>,
);

Expand All @@ -55,7 +54,7 @@ describe('App', () => {
expect(screen.queryByText('View more comments')).toBeInTheDocument();
fireEvent.click(screen.getByText('View more comments'));
expect(screen.queryByText('View more comments')).not.toBeInTheDocument();
expect(screen.getByText('Loading...')).toBeInTheDocument();
expect(screen.getByText('Display threads')).toBeInTheDocument();
});

it('should not render the comment form if user is logged out', async () => {
Expand All @@ -72,7 +71,6 @@ describe('App', () => {
expanded={false}
onPermalinkClick={() => {}}
apiKey=""
onHeightChange={() => {}}
/>,
);

Expand Down Expand Up @@ -100,7 +98,6 @@ describe('App', () => {
}}
apiKey="discussion-rendering-test"
onPermalinkClick={() => {}}
onHeightChange={() => {}}
/>,
);

Expand All @@ -127,7 +124,6 @@ describe('App', () => {
}}
apiKey="discussion-rendering-test"
onPermalinkClick={() => {}}
onHeightChange={() => {}}
/>,
);

Expand All @@ -153,7 +149,6 @@ describe('App', () => {
}}
apiKey="discussion-rendering-test"
onPermalinkClick={() => {}}
onHeightChange={() => {}}
/>,
);

Expand Down
19 changes: 2 additions & 17 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ type Props = {
expanded: boolean;
onPermalinkClick: (commentId: number) => void;
apiKey: string;
onHeightChange?: () => void;
onRecommend?: (commentId: number) => Promise<Boolean>;
onComment?: (shortUrl: string, body: string) => Promise<CommentResponse>;
onReply?: (
Expand Down Expand Up @@ -221,7 +220,6 @@ export const App = ({
expanded,
onPermalinkClick,
apiKey,
onHeightChange = () => {},
onRecommend,
onComment,
onReply,
Expand All @@ -236,7 +234,6 @@ export const App = ({
}),
);
const [isExpanded, setIsExpanded] = useState<boolean>(expanded);
const [rendering, setRendering] = useState<boolean>(false);
const [loading, setLoading] = useState<boolean>(true);
const [totalPages, setTotalPages] = useState<number>(0);
const [page, setPage] = useState<number>(initialPage || 1);
Expand Down Expand Up @@ -320,28 +317,16 @@ export const App = ({
rememberFilters(newFilterObject);
// Filters also show when the view is not expanded but we want to expand when they're changed
setIsExpanded(true);
onHeightChange();
setFilters(newFilterObject);
};

const onPageChange = (page: number) => {
document.getElementById('comment-filters')?.scrollIntoView();
setPage(page);
onHeightChange();
};

const expandView = () => {
setRendering(true);
onHeightChange();
// We use setTimeout here to push the isExpanded state change to the end of the
// stack. This has the effect of allowing the page to render with the new `rendering`
// state so that the button text updates. Only after that render is complete do we
// set isExpanded and display the remaining comments.
// Note. While its technically true that this slows down new comments appearing, the
// difference is negligable and worth it for the improved reader experience
setTimeout(() => {
setIsExpanded(true);
});
setIsExpanded(true);
};

const toggleMuteStatus = (userId: string) => {
Expand Down Expand Up @@ -465,7 +450,7 @@ export const App = ({
linkName="more-comments"
size="small"
>
{rendering ? 'Loading...' : 'View more comments'}
View more comments
</PillarButton>
</div>
)}
Expand Down

0 comments on commit c40232d

Please sign in to comment.