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: add support for design tokens and CSS variables #351

Closed
wants to merge 6 commits into from
Closed
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,7 @@ temp/babel-plugin-react-intl
### transifex ###
src/i18n/transifex_input.json
temp

### Local-only config ###
.env.private
module.config.js
21,444 changes: 12,814 additions & 8,630 deletions package-lock.json

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
"extends @edx/browserslist-config"
],
"dependencies": {
"@edx/brand": "npm:@edx/brand-openedx@^1.2.0",
"@edx/frontend-component-footer": "12.1.0",
"@edx/frontend-component-header": "4.3.0",
"@edx/frontend-platform": "4.6.0",
"@edx/paragon": "20.45.0",
"@edx/brand": "npm:@edx/brand-edx.org@2.2.0-alpha.16",
"@edx/frontend-component-footer": "12.2.1",
"@edx/frontend-component-header": "4.6.0",
"@edx/frontend-platform": "github:openedx/frontend-platform#ags/inject-theme-css",
"@edx/paragon": "22.0.0-alpha.1",
"@edx/react-unit-test-utils": "1.7.0",
"@edx/reactifex": "^2.1.1",
"@edx/reactifex": "^2.2.0",
"@fortawesome/fontawesome-svg-core": "^1.2.25",
"@fortawesome/free-brands-svg-icons": "^5.11.2",
"@fortawesome/free-solid-svg-icons": "^5.11.2",
Expand All @@ -54,8 +54,8 @@
"react-dom": "17.0.2",
"react-helmet": "^6.1.0",
"react-redux": "^7.2.9",
"react-router": "5.2.0",
"react-router-dom": "5.2.0",
"react-router": "6.15.0",
"react-router-dom": "6.15.0",
"react-router-redux": "^5.0.0-alpha.9",
"redux": "4.0.5",
"redux-beacon": "^2.1.0",
Expand All @@ -67,8 +67,8 @@
"whatwg-fetch": "^2.0.4"
},
"devDependencies": {
"@edx/browserslist-config": "^1.1.1",
"@edx/frontend-build": "12.9.3",
"@edx/browserslist-config": "^1.2.0",
"@edx/frontend-build": "github:openedx/frontend-build#ags/2321",
"@testing-library/react": "12.1.5",
"@wojtekmaj/enzyme-adapter-react-17": "0.8.0",
"axios": "0.21.2",
Expand Down
29 changes: 13 additions & 16 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { Route, Routes } from 'react-router-dom';

import { AppProvider } from '@edx/frontend-platform/react';

Expand All @@ -15,21 +15,18 @@ import Head from './head/Head';
const App = () => (
<AppProvider store={store}>
<Head />
<Router>
<div>
<Header />
<main>
<Switch>
<Route
exact
path={routePath}
component={GradebookPage}
/>
</Switch>
</main>
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
</div>
</Router>
<div>
<Header />
<main>
<Routes>
<Route
path={routePath}
element={<GradebookPage />}
/>
</Routes>
</main>
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
</div>
</AppProvider>
);

Expand Down
14 changes: 7 additions & 7 deletions src/App.scss
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
// frontend-app-*/src/index.scss
@import "~@edx/brand/paragon/fonts";
@import "~@edx/brand/paragon/variables";
@import "~@edx/paragon/scss/core/core";
@import "~@edx/brand/paragon/overrides";
//@import "~@edx/brand/paragon/fonts";
//@import "~@edx/brand/paragon/variables";
//@import "~@edx/paragon/scss/core/core";
//@import "~@edx/brand/paragon/overrides";

$fa-font-path: "~font-awesome/fonts";
@import "~font-awesome/scss/font-awesome";

$input-focus-box-shadow: $input-box-shadow; // hack to get upgrade to paragon 4.0.0 to work
$input-focus-box-shadow: var(--pgn-elevation-form-input-base); // hack to get upgrade to paragon 4.0.0 to work

@import "~@edx/frontend-component-header/dist/index";
@import "~@edx/frontend-component-footer/dist/_footer";
//@import "~@edx/frontend-component-header/dist/index";
//@import "~@edx/frontend-component-footer/dist/_footer";

@import "./components/GradesView/GradesView";
@import "./components/BulkManagementHistoryView/BulkManagementHistoryView";
Expand Down
23 changes: 11 additions & 12 deletions src/App.test.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { shallow } from 'enzyme';

import { BrowserRouter as Router, Route, Switch } from 'react-router-dom';
import { Route, Routes } from 'react-router-dom';
import { AppProvider } from '@edx/frontend-platform/react';

import Footer from '@edx/frontend-component-footer';
Expand All @@ -17,7 +17,6 @@ import Head from './head/Head';
jest.mock('react-router-dom', () => ({
BrowserRouter: () => 'BrowserRouter',
Route: () => 'Route',
Switch: () => 'Switch',
}));
jest.mock('@edx/frontend-platform/react', () => ({
AppProvider: () => 'AppProvider',
Expand All @@ -32,7 +31,7 @@ jest.mock('@edx/frontend-component-header', () => 'Header');

const logo = 'fakeLogo.png';
let el;
let router;
let secondChild;

describe('App router component', () => {
test('snapshot', () => {
Expand All @@ -42,7 +41,7 @@ describe('App router component', () => {
beforeEach(() => {
process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG = logo;
el = shallow(<App />);
router = el.childAt(1);
secondChild = el.childAt(1);
});
describe('AppProvider', () => {
test('AppProvider is the parent component, passed the redux store props', () => {
Expand All @@ -57,24 +56,24 @@ describe('App router component', () => {
});
describe('Router', () => {
test('second child of AppProvider', () => {
expect(router.type()).toBe(Router);
expect(secondChild.type()).toBe('div');
});
test('Header is above/outside-of the routing', () => {
expect(router.childAt(0).childAt(0).type()).toBe(Header);
expect(router.childAt(0).childAt(1).type()).toBe('main');
expect(secondChild.childAt(0).type()).toBe(Header);
expect(secondChild.childAt(1).type()).toBe('main');
});
test('Routing - GradebookPage is only route', () => {
expect(router.find('main')).toEqual(shallow(
expect(secondChild.find('main')).toEqual(shallow(
<main>
<Switch>
<Route exact path={routePath} component={GradebookPage} />
</Switch>
<Routes>
<Route path={routePath} element={<GradebookPage />} />
</Routes>
</main>,
));
});
});
test('Footer logo drawn from env variable', () => {
expect(router.find(Footer).props().logo).toEqual(logo);
expect(secondChild.find(Footer).props().logo).toEqual(logo);
});
});
});
27 changes: 12 additions & 15 deletions src/__snapshots__/App.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,17 @@ exports[`App router component snapshot 1`] = `
store="testStore"
>
<Head />
<BrowserRouter>
<div>
<Header />
<main>
<Switch>
<Route
component="GradebookPage"
exact={true}
path="/:courseId"
/>
</Switch>
</main>
<Footer />
</div>
</BrowserRouter>
<div>
<Header />
<main>
<Component>
<Route
element={<GradebookPage />}
path="/:courseId"
/>
</Component>
</main>
<Footer />
</div>
</AppProvider>
`;
2 changes: 1 addition & 1 deletion src/components/GradesView/GradebookTable/hooks.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const useGradebookTableData = () => {
[Headings.username]: (
<Fields.Username username={entry.username} userKey={entry.external_user_key} />
),
[Headings.email]: (<Fields.Email email={entry.email} />),
[Headings.email]: (<Fields.Text value={entry.email} />),
[Headings.totalGrade]: `${roundGrade(entry.percent * 100)}${getLocalizedPercentSign()}`,
...entry.section_breakdown.reduce((acc, subsection) => ({
...acc,
Expand Down
2 changes: 1 addition & 1 deletion src/components/GradesView/GradebookTable/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import useGradebookTableData from './hooks';
* a row for each user, with a column for their username, email, and total grade,
* along with one for each subsection in their grade entry.
*/
export const GradebookTable = () => {
const GradebookTable = () => {
const {
columns,
data,
Expand Down
2 changes: 1 addition & 1 deletion src/components/GradesView/GradesView.scss
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,6 @@ select#ScoreView.form-control {

&:before {
border-width: 0.4rem 0.4rem 0.4rem 0;
border-right-color: $black;
border-right-color: var(--pgn-color-black);
}
}
1 change: 1 addition & 0 deletions src/components/GradesView/ImportSuccessToast/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const ImportSuccessToast = () => {
show,
description,
} = useImportSuccessToastData();
return null;
return (
<Toast {...{ action, onClose, show }}>
{description}
Expand Down
23 changes: 10 additions & 13 deletions src/containers/GradebookPage/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import GradesView from 'components/GradesView';
import GradebookFilters from 'components/GradebookFilters';
import BulkManagementHistoryView from 'components/BulkManagementHistoryView';

import { withParams, withNavigate, withLocation } from '../../utils/hoc';

/**
* <GradebookPage />
* Top-level view for the Gradebook MFE.
Expand All @@ -28,10 +30,11 @@ export class GradebookPage extends React.Component {

componentDidMount() {
const urlQuery = queryString.parse(this.props.location.search);
this.props.initializeApp(this.props.match.params.courseId, urlQuery);
this.props.initializeApp(this.props.courseId, urlQuery);
}

updateQueryParams(queryParams) {
const { pathname } = this.props.location;
const parsed = queryString.parse(this.props.location.search);
Object.keys(queryParams).forEach((key) => {
if (queryParams[key]) {
Expand All @@ -40,7 +43,7 @@ export class GradebookPage extends React.Component {
delete parsed[key];
}
});
this.props.history.push(`?${queryString.stringify(parsed)}`);
this.props.navigate({ pathname, search: `?${queryString.stringify(parsed)}` });
}

render() {
Expand All @@ -60,18 +63,12 @@ export class GradebookPage extends React.Component {
}
}
GradebookPage.defaultProps = {
location: { search: '' },
location: { pathname: '/', search: '' },
};
GradebookPage.propTypes = {
history: PropTypes.shape({
push: PropTypes.func,
}).isRequired,
location: PropTypes.shape({ search: PropTypes.string }),
match: PropTypes.shape({
params: PropTypes.shape({
courseId: PropTypes.string,
}),
}).isRequired,
navigate: PropTypes.func.isRequired,
location: PropTypes.shape({ pathname: PropTypes.string, search: PropTypes.string }),
courseId: PropTypes.string.isRequired,
// redux
activeView: PropTypes.string.isRequired,
initializeApp: PropTypes.func.isRequired,
Expand All @@ -85,4 +82,4 @@ export const mapDispatchToProps = {
initializeApp: thunkActions.app.initialize,
};

export default connect(mapStateToProps, mapDispatchToProps)(GradebookPage);
export default connect(mapStateToProps, mapDispatchToProps)(withParams(withNavigate(withLocation(GradebookPage))));
11 changes: 6 additions & 5 deletions src/containers/GradebookPage/test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ describe('GradebookPage', () => {
let el;
const props = {
location: {
pathname: '/',
search: 'searchString',
},
match: { params: { courseId } },
courseId,
activeView: views.grades,
};
beforeEach(() => {
props.initializeApp = jest.fn();
props.history = { push: jest.fn() };
props.navigate = jest.fn();
});
test('snapshot - shows BulkManagementHistoryView if activeView === views.bulkManagementHistory', () => {
el = shallow(<GradebookPage {...props} activeView={views.bulkManagementHistory} />);
Expand Down Expand Up @@ -130,7 +131,7 @@ describe('GradebookPage', () => {
const val2 = 'VALTWO!!';
const args = { [newKey]: val1, [props.location.search]: val2 };
el.instance().updateQueryParams(args);
expect(props.history.push).toHaveBeenCalledWith(`?${queryString.stringify(args)}`);
expect(props.navigate).toHaveBeenCalledWith({ pathname: '/', search: `?${queryString.stringify(args)}` });
});
it('clears values for non-truthy values', () => {
queryString.parse.mockImplementation(key => ({ [key]: key }));
Expand All @@ -139,8 +140,8 @@ describe('GradebookPage', () => {
const val2 = false;
const args = { [newKey]: val1, [props.location.search]: val2 };
el.instance().updateQueryParams(args);
expect(props.history.push).toHaveBeenCalledWith(
`?${queryString.stringify({ [newKey]: val1 })}`,
expect(props.navigate).toHaveBeenCalledWith(
{ pathname: '/', search: `?${queryString.stringify({ [newKey]: val1 })}` },
);
});
});
Expand Down
24 changes: 24 additions & 0 deletions src/utils/hoc.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';

import { useLocation, useNavigate, useParams } from 'react-router-dom';

export const withParams = WrappedComponent => {
const WithParamsComponent = props => <WrappedComponent {...useParams()} {...props} />;
return WithParamsComponent;
};

export const withNavigate = WrappedComponent => {
const WithNavigateComponent = props => {
const navigate = useNavigate();
return <WrappedComponent navigate={navigate} {...props} />;
};
return WithNavigateComponent;
};

export const withLocation = WrappedComponent => {
const WithLocationComponent = props => {
const location = useLocation();
return <WrappedComponent location={location} {...props} />;
};
return WithLocationComponent;
};
Loading
Loading