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: allow full width content in library authoring [FC-0062] #1258

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
"@dnd-kit/utilities": "^3.2.2",
"@edx/brand": "npm:@openedx/brand-openedx@^1.2.3",
"@edx/browserslist-config": "1.2.0",
"@edx/frontend-component-footer": "^14.0.3",
"@edx/frontend-component-header": "^5.3.3",
"@edx/frontend-component-footer": "^14.1.0",
"@edx/frontend-component-header": "^5.5.0",
"@edx/frontend-enterprise-hotjar": "^2.0.0",
"@edx/frontend-platform": "^8.0.3",
"@edx/openedx-atlas": "^0.6.0",
Expand All @@ -65,7 +65,7 @@
"@openedx-plugins/course-app-xpert_unit_summary": "file:plugins/course-apps/xpert_unit_summary",
"@openedx/frontend-build": "^14.0.14",
"@openedx/frontend-plugin-framework": "^1.2.1",
"@openedx/paragon": "^22.5.1",
"@openedx/paragon": "^22.8.1",
"@redux-devtools/extension": "^3.3.0",
"@reduxjs/toolkit": "1.9.7",
"@tanstack/react-query": "4.36.1",
Expand Down
9 changes: 7 additions & 2 deletions src/header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,23 @@ import React from 'react';
import { getConfig } from '@edx/frontend-platform';
import { useIntl } from '@edx/frontend-platform/i18n';
import { StudioHeader } from '@edx/frontend-component-header';
import { useToggle } from '@openedx/paragon';
import { type Container, useToggle } from '@openedx/paragon';
import { generatePath, useHref } from 'react-router-dom';

import { SearchModal } from '../search-modal';
import { useContentMenuItems, useSettingMenuItems, useToolsMenuItems } from './hooks';
import messages from './messages';

type ContainerPropsType = React.ComponentProps<typeof Container>;

interface HeaderProps {
contextId?: string,
number?: string,
org?: string,
title?: string,
isHiddenMainMenu?: boolean,
isLibrary?: boolean,
containerProps?: ContainerPropsType,
}

const Header = ({
Expand All @@ -25,6 +28,7 @@ const Header = ({
title = '',
isHiddenMainMenu = false,
isLibrary = false,
containerProps = {},
}: HeaderProps) => {
const intl = useIntl();
const libraryHref = useHref('/library/:libraryId');
Expand Down Expand Up @@ -69,8 +73,9 @@ const Header = ({
mainMenuDropdowns={mainMenuDropdowns}
outlineLink={outlineLink}
searchButtonAction={meiliSearchEnabled ? openSearchModal : undefined}
containerProps={containerProps}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually want different pages/sections of this Authoring app to show a differently-sized header? This is probably a question for the CCs with oversight over the whole app, but I would think it's better to have these params here, and remove the containerProps from this Header class, so other pages don't mess with them.

Suggested change
containerProps={containerProps}
containerProps={{
size: undefined,
}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we actually want different pages/sections of this Authoring app to show a differently-sized header?

The idea came from here: #1221 (comment)
And there is a previous discussion here: #1217 (comment)
Personally, I prefer consistency throughout the app. But we would definitely use the extra space in the library authoring, and maybe changing everything would be a too big step.

/>
{ meiliSearchEnabled && (
{meiliSearchEnabled && (
<SearchModal
isOpen={isShowSearchModalOpen}
courseId={isLibrary ? undefined : contextId}
Expand Down
7 changes: 5 additions & 2 deletions src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,11 @@ const LibraryAuthoringPage = () => {
org={libraryData.org}
contextId={libraryId}
isLibrary
containerProps={{
size: undefined,
}}
/>
<Container size="xl" className="px-4 mt-4 mb-5 library-authoring-page">
<Container className="px-4 mt-4 mb-5 library-authoring-page">
<SearchContextProvider
extraFilter={`context_key = "${libraryId}"`}
>
Expand Down Expand Up @@ -218,7 +221,7 @@ const LibraryAuthoringPage = () => {
</Routes>
</SearchContextProvider>
</Container>
<StudioFooter />
<StudioFooter containerProps={{ size: undefined }} />
</div>
{ !!sidebarBodyComponent && (
<div className="library-authoring-sidebar box-shadow-left-1 bg-white" data-testid="library-sidebar">
Expand Down
13 changes: 2 additions & 11 deletions src/library-authoring/LibraryRecentlyModified.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useMemo } from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import { orderBy } from 'lodash';
import { CardGrid } from '@openedx/paragon';

import { SearchContextProvider, useSearchContext } from '../search-manager';
import { type CollectionHit, type ContentHit, SearchSortOption } from '../search-manager/data/api';
Expand Down Expand Up @@ -48,15 +47,7 @@ const RecentlyModified: React.FC<{ libraryId: string }> = ({ libraryId }) => {
title={intl.formatMessage(messages.recentlyModifiedTitle)}
contentCount={componentCount}
>
<CardGrid
columnSizes={{
sm: 12,
md: 6,
lg: 4,
xl: 3,
}}
hasEqualColumnHeights
>
<div className="library-cards-grid">
{recentItems.map((contentHit) => (
contentHit.type === 'collection' ? (
<CollectionCard
Expand All @@ -71,7 +62,7 @@ const RecentlyModified: React.FC<{ libraryId: string }> = ({ libraryId }) => {
/>
)
))}
</CardGrid>
</div>
</LibrarySection>
)
: null;
Expand Down
4 changes: 4 additions & 0 deletions src/library-authoring/components/ComponentCard.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
.library-component-card {
.pgn__card {
height: 100%
}

.library-component-header {
border-top-left-radius: .375rem;
border-top-right-radius: .375rem;
Expand Down
2 changes: 0 additions & 2 deletions src/search-manager/data/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ export async function fetchSearchResults({
highlightPreTag: HIGHLIGHT_PRE_TAG,
highlightPostTag: HIGHLIGHT_POST_TAG,
attributesToCrop: ['content'],
cropLength: 20,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed to the default (10) to reduce the text in the description. The design asked to limit to 3 lines, but we can only control word count here. We can fine-tune this in the AC test review.

sort,
offset,
limit,
Expand Down Expand Up @@ -281,7 +280,6 @@ export async function fetchSearchResults({
highlightPreTag: HIGHLIGHT_PRE_TAG,
highlightPostTag: HIGHLIGHT_POST_TAG,
attributesToCrop: ['description'],
cropLength: 15,
sort,
offset,
limit,
Expand Down