UI: Design a 404 page for default redirects #57

Aaishpra · 2020-08-29T12:49:40Z

Description

Solves the Following.

Design a 404 page
Change route URL to Redirect to /404 on wrong URLs

Fixes #33

Type of Change:

Code
User Interface

Code/Quality Assurance Only

New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

ScreenShot for Reference

Checklist:

My PR follows the style guidelines of this project
I have performed a self-review of my own code or materials
I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
Any dependent changes have been merged
I have written Kotlin Docs whenever is applicable

Code/Quality Assurance Only

My changes generate no new warnings
My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been published in downstream modules

Additional Context
I initially wanted to add a button to go back to the home page but since it doesn't Exist hence skipped that part. More suggestions and Additions are welcome for any other Feature within the page

bismitaguha · 2020-08-29T14:57:45Z

You should have used AuthRoutes IMO!

Aaishpra · 2020-08-30T03:07:21Z

Done @bismitaguha please review

bismitaguha · 2020-08-30T03:09:52Z

src/components/ErrorPage.js

+
+function ErrorPage() {
+    return (
+        <>    <Navbar /> 


AuthRoute already contains the Navbar, so this wasn't needed. Reusing components is always preferable. That's why I suggested AuthRoute.

bismitaguha · 2020-08-30T03:10:36Z

src/components/ErrorPage.js

+    )
+}
+
+export default ErrorPage


Throughout the application class method has been used.

@Aaishpra Make these changes

Already done @bismitaguha

@bismitaguha is this correct now

Aaishpra · 2020-08-31T06:18:40Z

Changed the files @bismitaguha

Aaishpra · 2020-09-01T10:26:29Z

@bismitaguha please review the changes

Aaishpra · 2020-09-02T09:08:52Z

@abha224 can u review the pull request as @bismitaguha seems not to be available

bismitaguha · 2020-09-02T09:43:32Z

I had requested changes @Aaishpra .Please make them!

Aaishpra · 2020-09-02T09:47:29Z

@bismitaguha can u please check my latest commits i guess i changed the file as u requested.

bismitaguha · 2020-09-02T09:59:27Z

@Aaishpra I didn't ask you to remove classes. The code already written is in exporting a class component not by exporting default function methods
Just go through the src/ directory once.

sidvenu · 2020-09-02T10:28:39Z

@Aaishpra is the 404 text color the same as the color of Open Source Programs header? If not, I suggest switching to that color.

Aaishpra · 2020-09-02T13:43:23Z

@sidvenu the color is the same maybe it looks a bit different because the text 404 is much bolder than Open source programs header.

Aaishpra · 2020-09-02T13:44:53Z

@bismitaguha is the latest commit ok now?

Aaishpra · 2020-09-04T16:14:50Z

@bismitaguha please review the changes

bismitaguha · 2020-09-05T07:23:45Z

@Aaishpra Either me or someone else will surely review your PR, from time to time. Considering my situation, I am running busy lately, and so are many other members of the community too. Please have patience and don't worry at all. Your PRs will surely be reviewed. Although, if a week or more passes surely ping/mention any of us on Github/Zulip.

Aaishpra · 2020-09-05T08:05:14Z

No Problem @bismitaguha got your point.

Aaishpra · 2020-09-09T06:19:25Z

@sidvenu can you please review this.

sidvenu

LGTM!

sidvenu · 2020-09-11T03:48:15Z

@anitab-org/qa-team @anitab-org/open-source-progs-maintainers @anitab-org/open-source-progs-developers Can anyone test this PR out?

Aaishpra · 2020-09-21T12:37:56Z

Hey,@bismitaguha and @sidvenu can you suggest someone that can test this PR out so that i can personally tag them here, or you people can help with it. Actually a lot of time has already passed since the last review, and no one has come to see this here.

sidvenu · 2020-09-23T12:57:46Z

@rpattath @bismitaguha if possible, could you test this PR?

devkapilbansal

@Aaishpra please squash your commits into one. It's always better to have one commit per change to keep the history clean

devkapilbansal · 2020-10-20T09:16:04Z

src/styles/ErrorPage.css

+
+

Why two blank lines are here? We only need one

INNOVATIVEGAMER · 2021-01-24T16:21:46Z

@Aaishpra You added custom css for all your styling. Styling is promising but this project is already using Semantic-UI as UI framework. I added some comments inside your css with some semantic-ui links which you can use in your component.
You can still use your custom one or two line css by adding style={{}} in components prop directly.
@isabelcosta I highly recommend you to add guidelines for usage of semantic-ui in designing of this repo.

isabelcosta · 2021-01-24T16:30:11Z

@INNOVATIVEGAMER good point, could you please create a issue for us to add this to the guidelines please?
@Aaishpra could you take a look at this. How feasible it is to use this instead of custom styling? https://semantic-ui.com/

INNOVATIVEGAMER · 2021-01-24T18:15:46Z

@INNOVATIVEGAMER good point, could you please create a issue for us to add this to the guidelines please?

I created the issue for this #65. You can check on that.
Also the contributing guidelines link is not working in readme with some mistakes, I created the issue for same also in #64 .

devkapilbansal

@vj-codes @isabelcosta I am not sure about coding practices and all.
But when I tested it locally, the UI changes looks good to me

The changes made in this PR is tested locally, following are the results:-

Code Review - Not Done
All possible responses were tested as below:

Test1 Description - 404 error page appears when urls are wrong
Expected Result:- The error page should appear
Actual Result:- Same as expected
Test2 Description - The UI should be responsive
Actual Result same as expected

isabelcosta · 2021-01-24T21:24:01Z

@devkapilbansal that is just awesome! thank you so much!

Aaishpra · 2021-01-25T05:34:55Z

@Aaishpra You added custom css for all your styling. Styling is promising but this project is already using Semantic-UI as UI framework. I added some comments inside your css with some semantic-ui links which you can use in your component.
You can still use your custom one or two line css by adding style={{}} in components prop directly.
@isabelcosta I highly recommend you to add guidelines for usage of semantic-ui in designing of this repo.

Thanks for the suggestion, I will update soon with the changes.

Merge branch 'Aaishpra-patch-1' of https://github.com/Aaishpra/open-source-programs-web into Aaishpra-patch-1

Aaishpra · 2021-02-05T02:53:22Z

@isabelcosta I have modified the files according to the suggestion, without any change in designs. Here, I would like to mention one thing earlier I tried using <Container Text> from semantic-UI but it was breaking the design without centering the content, so I still have used some of my custom CSS to center the content and keep the designs same.

Hope this is good.

codesankalp

Tested Locally @isabelcosta @Aaishpra @vj-codes

Details of the test mentioned below:
Code-Review: Not Done.
Tests:

The 404 error page for invalid URL when not logged in.
Screenshot:

404 Error page not showing when logged in it redirects to home page maybe this is due to AuthRoute.
GIF:
The scaling works perfect but for smaller width and height I observed this:
Screenshot:

Also, Please squash your commits @Aaishpra .

isabelcosta · 2021-02-05T19:15:31Z

I can squash so no need to squash @Aaishpra :)
@devkapilbansal and @codesankalp can you please add a code review?
@Aaishpra can you try to get more reviews from the community please, when it has 2 reviews I can merge :)

Rahulm2310 · 2021-02-05T19:55:51Z

src/styles/ErrorPage.css

+}
+
+.errorPage {
+    position: absolute;


@Aaishpra Can I ask why are you using absolute positioning when this can be achieved with relative positioning as well. This way the overlapping issue on small screens mentioned by @codesankalp will also resolve.

relative is not gonna work on larger screen

Aaishpra · 2021-02-06T03:29:48Z

This is what i see on my screen when changing position from absolute to relative (also the current design is working fine for smaller heights and width as tested by @devkapilbansal and i don't know how to make it work for even smaller widths)

about the AuthRoute part using of AuthRoute was suggested to me by @bismitaguha during code review of this PR (and i also think that it's the correct way to do the redirect instead of using Route).

cc : @isabelcosta @codesankalp @Rahulm2310 @devkapilbansal

Rahulm2310 · 2021-02-06T03:43:34Z

@Aaishpra Not just doing position : relative would work. Try adding width :100% height:100vh. As you already have display flex, justify-content:center, align-items:center, I think this should work. 🙂

Aaishpra · 2021-02-06T03:48:02Z

@Aaishpra Not just doing position : relative would work. Try adding width :100% height:100vh. As you already have display flex, justify-content:center, align-items:center, I think this should work. slightly_smiling_face

Not working

khushishikhu · 2021-02-06T06:46:20Z

I have tested the PR. LGTM!! keep it up @Aaishpra

codesankalp · 2021-02-06T08:28:48Z

src/styles/ErrorPage.css

+.errorPage_content p {
+    font-size: 1.4em;
+    color: gray;
+}


Can we use className (err-num and err-text) instead of style={{}} for Header in ErrorPage.js and add the styles in ErrorPage.css as mentioned below?

Suggested change

}

}

.ui.header.err-num {

color: #2185D0;

font-size: 10vw;

font-weight: bolder;

line-height: 1em;

text-transform: uppercase;

}

.ui.header.err-text {

font-size: 2em;

margin-bottom: 20px;

text-transform: uppercase;

max-width: 600px;

color: #0d0d0d;

}

Any specific reason @codesankalp ? Is this a common convention?

codesankalp

Code review is done.
LGTM 👍

isabelcosta · 2021-02-06T10:40:15Z

Thank you all for the reviews :) I will merge it, and some comments about styling can be done at a later issue.

bismitaguha suggested changes Aug 30, 2020

View reviewed changes

Aaishpra requested a review from bismitaguha August 30, 2020 05:33

bismitaguha previously approved these changes Sep 5, 2020

View reviewed changes

sidvenu previously approved these changes Sep 9, 2020

View reviewed changes

sidvenu added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Category: User Interface Improvements or additions to design. Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Sep 11, 2020

Aaishpra mentioned this pull request Oct 14, 2020

Coding : Developed the New To Open Source Section of Contribute Page anitab-org/anitab-org.github.io#146

Merged

10 tasks

devkapilbansal suggested changes Oct 20, 2020

View reviewed changes

devkapilbansal reviewed Oct 20, 2020

View reviewed changes

src/styles/ErrorPage.css Outdated

Comment on lines 38 to 39

Copy link

Member

devkapilbansal Oct 20, 2020

Choose a reason for hiding this comment

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

Why two blank lines are here? We only need one

rpattath added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Oct 21, 2020

devkapilbansal reviewed Jan 24, 2021

View reviewed changes

Fork update

81f7d81

Merge branch 'Aaishpra-patch-1' of https://github.com/Aaishpra/open-source-programs-web into Aaishpra-patch-1

isabelcosta mentioned this pull request Feb 4, 2021

As an User, I am able to fill, edit and view the form #10

Open

7 tasks

Aaishpra added 2 commits February 5, 2021 08:08

semantic ui used

baa1bca

formatted document

dce8022

Aaishpra dismissed stale reviews from isabelcosta and vj-codes via dce8022 February 5, 2021 02:40

Container removed

1680cef

Aaishpra requested review from isabelcosta and vj-codes and removed request for sidvenu and bismitaguha February 5, 2021 02:53

codesankalp reviewed Feb 5, 2021

View reviewed changes

Rahulm2310 reviewed Feb 5, 2021

View reviewed changes

khushishikhu approved these changes Feb 6, 2021

View reviewed changes

codesankalp reviewed Feb 6, 2021

View reviewed changes

codesankalp approved these changes Feb 6, 2021

View reviewed changes

isabelcosta merged commit ab18727 into anitab-org:develop Feb 6, 2021

-}
+}
+.ui.header.err-num {
+    color: #2185D0;
+    font-size: 10vw;
+    font-weight: bolder;
+    line-height: 1em;
+    text-transform: uppercase;
+}
+.ui.header.err-text {
+    font-size: 2em;
+    margin-bottom: 20px;
+    text-transform: uppercase;
+    max-width: 600px;
+    color: #0d0d0d;
+}

UI: Design a 404 page for default redirects #57

UI: Design a 404 page for default redirects #57

Conversation

Aaishpra commented Aug 29, 2020 • edited Loading

Description

Type of Change:

How Has This Been Tested?

Checklist:

bismitaguha commented Aug 29, 2020

Aaishpra commented Aug 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaishpra commented Aug 31, 2020

Aaishpra commented Sep 1, 2020

Aaishpra commented Sep 2, 2020

bismitaguha commented Sep 2, 2020 • edited Loading

Aaishpra commented Sep 2, 2020

bismitaguha commented Sep 2, 2020

sidvenu commented Sep 2, 2020

Aaishpra commented Sep 2, 2020 • edited Loading

Aaishpra commented Sep 2, 2020 • edited Loading

Aaishpra commented Sep 4, 2020

bismitaguha commented Sep 5, 2020

Aaishpra commented Sep 5, 2020

Aaishpra commented Sep 9, 2020 • edited Loading

sidvenu left a comment

Choose a reason for hiding this comment

sidvenu commented Sep 11, 2020

Aaishpra commented Sep 21, 2020

sidvenu commented Sep 23, 2020

devkapilbansal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

INNOVATIVEGAMER commented Jan 24, 2021

isabelcosta commented Jan 24, 2021

INNOVATIVEGAMER commented Jan 24, 2021

devkapilbansal left a comment

Choose a reason for hiding this comment

isabelcosta commented Jan 24, 2021

Aaishpra commented Jan 25, 2021

Aaishpra commented Feb 5, 2021

codesankalp left a comment • edited Loading

Choose a reason for hiding this comment

isabelcosta commented Feb 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaishpra commented Feb 6, 2021

Rahulm2310 commented Feb 6, 2021

Aaishpra commented Feb 6, 2021

khushishikhu commented Feb 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codesankalp left a comment

Choose a reason for hiding this comment

isabelcosta commented Feb 6, 2021

Aaishpra commented Aug 29, 2020 •

edited

Loading

bismitaguha commented Sep 2, 2020 •

edited

Loading

Aaishpra commented Sep 2, 2020 •

edited

Loading

Aaishpra commented Sep 2, 2020 •

edited

Loading

Aaishpra commented Sep 9, 2020 •

edited

Loading

codesankalp left a comment •

edited

Loading