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

ATL-10: Add catch-all 404 route/page #4

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

cesarvh
Copy link

@cesarvh cesarvh commented Mar 14, 2023

src/router.ts Outdated
},
{
path: '/',
component: DefaultVue,
Copy link
Contributor

Choose a reason for hiding this comment

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

@cesarvh - not a merge blocker but perhaps we want DefaultView instead of DefaultVue?

@@ -0,0 +1,3 @@
.frosted {
background-color: #6d8cb4c3!important;
Copy link
Contributor

Choose a reason for hiding this comment

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

@cesarvh - That seems to be an invalid hex code. And, a space is needed after the hex code.

:height="300"
:width="800"
class="frosted px-8 py-6 accent-border accent-text mx-auto "
>
Copy link
Contributor

Choose a reason for hiding this comment

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

@cesarvh - it seems that your "continuation indentation" rule is four spaces. I think it should be two spaces. You can enforce such rule via .eslintrc.js

@@ -1,5 +1,5 @@
<template>
<v-container class="fill-height" style="">
<v-container class="fill-height">
<v-responsive class="d-flex align-center text-center fill-height ">
Copy link
Contributor

Choose a reason for hiding this comment

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

We have an extra space in 'class' value above.

@johncrossman johncrossman merged commit 6348599 into ets-berkeley-edu:main Mar 14, 2023
gmerritt added a commit that referenced this pull request Feb 16, 2024
Merge pull request #17 from gmerritt/main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants