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.

Sign up for GitHub

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

Jump to bottom

feat: allow full width content in library authoring [FC-0062] #1258

Merged
merged 7 commits into from
Oct 2, 2024
Merged

feat: allow full width content in library authoring [FC-0062] #1258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99da9c7
feat: allow full width content in library authoring
rpenido Sep 18, 2024
9252b6e
fix: typo on css
rpenido Sep 20, 2024
b2a4cbf
Merge branch 'master' into rpenido/fal-3820-allow-full-width-content
rpenido Oct 1, 2024
d5e4f5f
chore: update header and footer
rpenido Oct 1, 2024
cbc1661
fix: reduce cropLength to the default 10
rpenido Oct 1, 2024
d50c109
fix: eslint
rpenido Oct 1, 2024
f08d932
fix: recently modified grid
rpenido Oct 1, 2024
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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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