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

Introduce Error 404 page #1107

Merged
merged 13 commits into from
Jan 16, 2025
7 changes: 6 additions & 1 deletion _dev/src/scss/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ $cdk-prefix: "cdk-";
0.15s
);
--#{$ua-prefix}header-height: var(--#{$cdk-prefix}header-height, 3.25rem);
--#{$ua-prefix}header-offset: 10rem;
--#{$ua-prefix}header-offset: 11.25rem;
--#{$ua-prefix}header-offset-mobile: calc(var(--#{$ua-prefix}header-offset) - 1rem);
--#{$ua-prefix}bo-background-color: var(--#{$ua-prefix}primary-200);
--#{$ua-prefix}error-img-filter: none;
--#{$ua-prefix}scrollbar-thumb: var(--#{$ua-prefix}primary-500);
Expand All @@ -70,13 +71,17 @@ body {
--#{$ua-prefix}error-img-filter: invert(69%) sepia(66%) saturate(1340%) hue-rotate(151deg)
brightness(88%) contrast(91%);
--#{$ua-prefix}header-height: 2.5rem;
--#{$ua-prefix}header-offset: 9rem;
--#{$ua-prefix}header-offset-mobile: calc(var(--#{$ua-prefix}header-offset) - 0.75rem);
}

&:has(.v1-7-3-0) {
--#{$ua-prefix}bo-background-color: #eff1f2;
--#{$ua-prefix}error-img-filter: invert(75%) sepia(64%) saturate(6023%) hue-rotate(161deg)
brightness(96%) contrast(103%);
--#{$ua-prefix}header-height: 2.5rem;
--#{$ua-prefix}header-offset: 10rem;
--#{$ua-prefix}header-offset-mobile: calc(var(--#{$ua-prefix}header-offset) + 1rem);
}
}

Expand Down
41 changes: 36 additions & 5 deletions _dev/src/scss/layouts/_error.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ $e: ".error-page";
&__img {
max-width: 100%;
height: auto;
filter: var(--#{$ua-prefix}error-img-filter);
}

&__infos {
Expand Down Expand Up @@ -67,29 +66,61 @@ $e: ".error-page";
&__buttons {
flex-direction: column;
}

&__button {
justify-content: center;
}
}
}

// Handle auto fit height
@container html-height (min-width: 0) {
@container html-block (min-width: 0) {
display: grid;
grid-template-rows: 1fr;
grid-template-columns: minmax(0, 1fr);
min-height: calc(100cqh - var(--#{$ua-prefix}header-offset-mobile));
}

@container html-block (min-width: 1024px) {
min-height: calc(100cqh - var(--#{$ua-prefix}header-offset));
}
}

// Handle auto fit height
// Enable auto fit height
html {
&:has(#{$ua-id}) {
&:has(.error-page) {
&:has(#{$e}) {
container-type: size;
container-name: html-height;
container-name: html-block;

#{$ua-id} {
&:not(.v1-7-8-0):not(.v1-7-3-0) {
#{$e} {
padding-block-end: 0;
}
}
}

#{$e} {
display: grid;
height: 100%;
container-type: inline-size;
container-name: ua-error;
padding-block-end: 0.75rem;
}

// Custom styles to adapt PrestaShop Back Office
body {
overflow-y: auto;
background-color: var(--#{$ua-prefix}bo-background-color);

#nav-sidebar {
height: calc(100% - var(--#{$ua-prefix}header-height));
}
}

#main {
padding-block-end: 0;
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions _dev/src/scss/layouts/_layout.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ $e: "#ua";
#{$ua-id} {
// Default BO theme layout fix
padding-inline-end: 5px;

&:not(.v1-7-8-0):not(.v1-7-3-0) {
padding-inline-end: 0;
}
}

#{$e} {
Expand Down
19 changes: 11 additions & 8 deletions _dev/src/scss/layouts/_logs-pages.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ html {
// Need class use in #ua_step_content to make it work
&:has(.update-page, .restore-page) {
container-type: size;
container-name: html-height;
container-name: html-block;

// Custom styles to adapt PrestaShop Back Office
body {
overflow-y: auto;
background-color: var(--#{$ua-prefix}bo-background-color);
Expand All @@ -28,19 +29,15 @@ html {
// Logs adjustments
.logs {
flex-grow: 1;

&__scroll {
container-type: size;
}
}

// Page adjustments
@container html-height (min-width: 0) {
// Page adjustments to handle auto fit height
@container html-block (min-width: 0) {
#{$ua-id} {
display: grid;
grid-template-rows: 1fr;
grid-template-columns: minmax(0, 1fr);
min-height: calc(100cqh - var(--#{$ua-prefix}header-offset));
min-height: calc(100cqh - var(--#{$ua-prefix}header-offset-mobile));

#ua_container {
flex-grow: 1;
Expand All @@ -59,6 +56,12 @@ html {
}
}
}

@container html-block (min-width: 1024px) {
#{$ua-id} {
min-height: calc(100cqh - var(--#{$ua-prefix}header-offset));
}
}
}
}
}
6 changes: 6 additions & 0 deletions _dev/src/scss/layouts/_welcome.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
$e: ".welcome-page";

#{$ua-id} {
&:has(.welcome-page) {
#ua_main {
padding-block-start: 0;
}
}

#{$e} {
&__card-list {
display: flex;
Expand Down
17 changes: 13 additions & 4 deletions _dev/src/ts/api/RequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,25 @@ export class RequestHandler {
data.append('dir', window.AutoUpgradeVariables.admin_dir);

try {
const response = await baseApi.post('', data, {
const response = await baseApi.post<ApiResponse>('', data, {
params: { route },
signal
});

const responseData = response.data as ApiResponse;
const responseData = response.data;
await this.#handleResponse(responseData, fromPopState);
} catch (error) {
// TODO: catch errors
console.error(error);
// A couple or errors are returned in an actual response (i.e 404 or 500)
if (error instanceof AxiosError) {
if (error.response?.data) {
const responseData = error.response.data;
responseData.new_route = 'error-page';
await this.#handleResponse(responseData, true);
}
} else {
// TODO: catch errors
console.error(error);
}
Comment on lines +43 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

Axios allows the use of interceptor, which allows specific behavior based on an error code. This would allow us to easily control/orient the view used and potentially have logic on a specific error.

Ex:

axiosInstance.interceptors.response.use(undefined, (error) => {
  const { response } = error;
  if (!response) {
    history.push(ERROR_UNKNOWN);
  } else {
    const { status } = response;

    switch (status) {
      case 409:
        break;
      case 404:
        history.push(ERROR_NOT_FOUND);
        break;
      case 401:
        history.push(ERROR_UNAUTHORIZED);
        break;
      case 403:
        history.push(ERROR_FORBIDDEN);
        break;
      case 500:
      case 503:
      case 400:
      default:
        history.push(ERROR_UNKNOWN);
    }
  }
  return Promise.reject(error);
});

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea of using the interceptor, especially to validate the response contents from the back end.

I suggest to handle this in another pull-request though, as we only cover the HTTP 404 here.

}
}

Expand Down
23 changes: 23 additions & 0 deletions _dev/src/ts/pages/ErrorPage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import DomLifecycle from '../types/DomLifecycle';
import ErrorPage404 from './error/ErrorPage404';
import PageAbstract from './PageAbstract';

export default class ErrorPage extends PageAbstract {
errorPage?: DomLifecycle;

constructor() {
super();

if (document.getElementById('ua_error_404')) {
this.errorPage = new ErrorPage404();
}
}

public mount = (): void => {
this.errorPage?.mount();
};

public beforeDestroy = (): void => {
this.errorPage?.beforeDestroy();
};
}
53 changes: 53 additions & 0 deletions _dev/src/ts/pages/error/ErrorPage404.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import api from '../../api/RequestHandler';
import DomLifecycle from '../../types/DomLifecycle';

export default class ErrorPage404 implements DomLifecycle {
isOnHomePage: boolean = false;

public constructor() {
this.isOnHomePage = new URLSearchParams(window.location.search).get('route') === 'home-page';
}

public mount = (): void => {
this.#activeActionButton.classList.remove('hidden');
this.#form.addEventListener('submit', this.#onSubmit);
};

public beforeDestroy = (): void => {
this.#form.removeEventListener('submit', this.#onSubmit);
};

get #activeActionButton(): HTMLFormElement | HTMLAnchorElement {
return this.isOnHomePage ? this.#exitButton : this.#form;
}

get #form(): HTMLFormElement {
const form = document.forms.namedItem('home-page-form');
if (!form) {
throw new Error('Form not found');
}

['routeToSubmit'].forEach((data) => {
if (!form.dataset[data]) {
throw new Error(`Missing data ${data} from form dataset.`);
}
});

return form;
}

get #exitButton(): HTMLAnchorElement {
const link = document.getElementById('exit-button');

if (!link || !(link instanceof HTMLAnchorElement)) {
throw new Error('Link is not found or invalid');
}
return link;
}

readonly #onSubmit = async (event: Event): Promise<void> => {
event.preventDefault();

await api.post(this.#form.dataset.routeToSubmit!, new FormData(this.#form));
};
}
11 changes: 9 additions & 2 deletions _dev/src/ts/routing/ScriptHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import SendErrorReportDialog from '../dialogs/SendErrorReportDialog';

import { ScriptType, ScriptsMatching, CurrentScripts } from '../types/scriptHandlerTypes';
import { routeHandler } from '../autoUpgrade';
import ErrorPage from '../pages/ErrorPage';

export default class ScriptHandler {
#currentScripts: CurrentScripts = {
Expand All @@ -41,7 +42,9 @@ export default class ScriptHandler {

'restore-page-backup-selection': RestorePageBackupSelection,
'restore-page-restore': RestorePageRestore,
'restore-page-post-restore': RestorePagePostRestore
'restore-page-post-restore': RestorePagePostRestore,

'error-page': ErrorPage
},
[ScriptType.DIALOG]: {
'restore-backup-dialog': RestoreBackupDialog,
Expand Down Expand Up @@ -69,10 +72,14 @@ export default class ScriptHandler {
* @returns void
* @description Loads and mounts the script associated with the specified script name.
*/
public loadScript(scriptID: string) {
public loadScript(scriptID: string): void {
const scriptType = this.#getScriptTypeByScriptID(scriptID);

if (!scriptType) {
console.debug(`No matching class found for ID: ${scriptID}`);
// Outside a hydration, the scriptID matches the route query param.
// If it does not exist, we load the error management script instead.
this.loadScript('error-page');
return;
}

Expand Down
9 changes: 8 additions & 1 deletion classes/Router/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

namespace PrestaShop\Module\AutoUpgrade\Router;

use PrestaShop\Module\AutoUpgrade\Controller\Error404Controller;
use PrestaShop\Module\AutoUpgrade\Controller\ErrorReportController;
use PrestaShop\Module\AutoUpgrade\Controller\HomePageController;
use PrestaShop\Module\AutoUpgrade\Controller\LogsController;
Expand Down Expand Up @@ -223,6 +224,11 @@ public function __construct(UpgradeContainer $upgradeContainer)
'controller' => LogsController::class,
'method' => 'getDownloadLogsButton',
],

Routes::ERROR_404 => [
'controller' => Error404Controller::class,
'method' => 'index',
],
];

/**
Expand All @@ -232,7 +238,8 @@ public function __construct(UpgradeContainer $upgradeContainer)
*/
public function handle(Request $request)
{
$route = self::ROUTES[$request->query->get('route')] ?? self::ROUTES[Routes::HOME_PAGE];
$routeName = $request->query->get('route') ?? Routes::HOME_PAGE;
$route = self::ROUTES[$routeName] ?? self::ROUTES[Routes::ERROR_404];

$method = $route['method'];

Expand Down
3 changes: 3 additions & 0 deletions classes/Router/Routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@ class Routes

/* logs */
const DOWNLOAD_LOGS = 'download-logs';

/* errors */
const ERROR_404 = 'error-404';
}
Loading
Loading