Skip to content

Feat/add a help feature - #166

Merged
gbowne1 merged 11 commits into
gbowne1:masterfrom
BlackBond06:Feat/Add-a-help-feature
Sep 10, 2023
Merged

Feat/add a help feature#166
gbowne1 merged 11 commits into
gbowne1:masterfrom
BlackBond06:Feat/Add-a-help-feature

Conversation

@BlackBond06

Copy link
Copy Markdown
Collaborator

This PR implements the Help/ Support feature of the code booker app, issue #123, opened by @gbowne1.

Currently this feature allows users to send help/ support query, through a form provided on the app, to the codebooker support mail team and also, stores this query in the db for proper management.

The following changes were made:

  • Created a Support component page
  • Created the support schema model
  • Created the new support API endpoint
  • Implemented the newSupport controller function for handling support query submissions and database storage.

The following dependencies were used:

  • Formik for form state handling
  • Yup for form inputs validation.
  • nodemailer for email submisssion.
React.App.-.Google.Chrome.2023-09-09.14-21-41.mp4

pic

@BlackBond06
BlackBond06 requested a review from gbowne1 as a code owner September 9, 2023 13:24
@gbowne1
gbowne1 requested a review from LOGESH-B September 9, 2023 17:14
@gbowne1 gbowne1 added frontend Work on Frontend feature request this is a feature request code-review This is ready for code review labels Sep 9, 2023
@gbowne1 gbowne1 linked an issue Sep 9, 2023 that may be closed by this pull request
@gbowne1 gbowne1 added this to the Frontend milestone Sep 9, 2023
@gbowne1

gbowne1 commented Sep 9, 2023

Copy link
Copy Markdown
Owner

@BlackBond06 This has a merge conflict with App.js. Please review App.js and commit the fixes. Won't be able to merge till this is fixed.

I think it has to do with incoming changes. The === and <<< show where the issues are.

import * as React from 'react';
import Home from './pages/Home/Home';
import { Route, Routes } from 'react-router-dom';
import { BrowserRouter as Router } from 'react-router-dom';
import Login from './pages/Login/Login';
import Profile from './pages/Profile/Profile';
import Register from './components/Register/Register';
import UserAuthenticated from './ProtectedRoute';
import ForgotPassword from './pages/ForgotPassword/ForgotPassword';
import ResetPassword from './pages/ResetPassword/ResetPassword';
<<<<<<< Feat/Add-a-help-feature
import Support from './pages/Support/Support';
=======
import PrivacyPolicy from './pages/PrivacyPolicy/PrivacyPolicy';
>>>>>>> master

export default function App() {
    return (
        <Router>
            <Routes>
                <Route element={<UserAuthenticated />}>
                    <Route path='/' element={<Home />} />
<<<<<<< Feat/Add-a-help-feature
                    <Route path='/profile/:uid' element={<Profile />} />
                    <Route path='/support' element={<Support />} />
=======
                    <Route path='/profile' element={<Profile />} />
                    <Route path='/privacy-policy' element={<PrivacyPolicy />} />
>>>>>>> master
                </Route>
                <Route path='/login' element={<Login />} />
                <Route path='/register' element={<Register />} />
                <Route path='/forgot-password' element={<ForgotPassword />} />
                <Route
                    path='/reset-password/:token'
                    element={<ResetPassword />}
                />
            </Routes>
        </Router>
    );
}

@gbowne1 gbowne1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

  • Checked out this change locally for testing and review for merge
  • Manually reviewed code for grammar, spelling and other errors
  • Reviewed in broswer for additional warnings, errors

This has a merge conflict with App.js with the incoming/outgoing changes.

This change PR implements a support/help feature added to the User context dropdown menu under the (R) user icon.

Because of the merge conflict with App.js I had to manually adopt changes.

It does not appear to cause any additional warnings in browser, but more review in browser may need to happen since this is an extensive feature.

Approving this for merge, but with requested changes.

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

Ok I will make the changes to resolve this merge conflict now.

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

Hi @gbowne1, I have resolved the merge conflict.

@gbowne1 gbowne1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks like these changes fixed the merge conflict.

Approving for merge. Excellent work!

Thank you for the opportunity to review your PR! Looking forwards to reviewing more of your excellent PR's.

@LOGESH-B LOGESH-B left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hii @BlackBond06
Thanks for your contribution....
Tested Locally Approving for merge

On my system UI is not properly aligned
image

@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

@logesh, a few of the recent changes and added features may need some additional styling features, as well as mobile styling. There's a #79 issue which someone had been working on.. but I think it's an abandoned issue.

We also have some outlying issues with the App.css & index.css causing some issues with Firefox due to webkit and moz kit. I would suggest modifying the default global styling in App.css and index.css

@BlackBond06 @victor20m @ratishjain12

@gbowne1
gbowne1 merged commit 8f4e330 into gbowne1:master Sep 10, 2023
@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

@LOGESH-B can you go here https://whatismyviewport.com/ and let us know what your viewport size is?

@LOGESH-B

Copy link
Copy Markdown
Collaborator

image

@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

laptop?

@LOGESH-B

Copy link
Copy Markdown
Collaborator

yeah @gbowne1

@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

Then I suggest we work on mobile styling at some point very soon and test out the breakpoints we can also @media-query alternative breakpoints As I mentioned earlier there is a #79 issue for Mobile styling.

oh and I answered your discussion.

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

Yea @LOGESH-B, I just noticed the UI problem when I pulled the recent changes from the upstream. Can't really what the problem is. Any ideas?

@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

@BlackBond06 if you use Firefox or Chrome, you can set the responsive view in console to his view 1536x739 and see how it looks then just modify it till it looks proper in that view.

This happens occasionally, and can be worked around with a media query.

A media query in the affected portions of the CSS with some minor nudging of some of the CSS selectors in the media query. Making some custom queried CSS for laptops (which are usually around 1500px x700px) might fix this.

I alwas suggest people look at https://whatismyviewport.com/ and note their viewport sizes while working on this.

Mine is 1884px x 905px

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

Okay thanks, I will make the UI changes to fix this.

@gbowne1

gbowne1 commented Sep 10, 2023

Copy link
Copy Markdown
Owner

You can always make a new issue to cover this change. I or @LOGESH-B could assign you.

@BlackBond06

Copy link
Copy Markdown
Collaborator Author

@gbowne1, @LOGESH-B.
I have made the change to fix the UI issue on the Help/Support feature as requested. Pls review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-review This is ready for code review feature request this is a feature request frontend Work on Frontend

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[FEAT] Add a Help feature

3 participants