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

Secondary Dropdown #808

Merged
merged 14 commits into from
Apr 29, 2020
Merged

Secondary Dropdown #808

merged 14 commits into from
Apr 29, 2020

Conversation

meghanacosmos
Copy link
Contributor

@meghanacosmos meghanacosmos commented Apr 22, 2020

related to issue #560
Component can be viewed at /interface/secondaryDropdown
To test this PR, start searching with H in the input to view the dropdown.
As of now, these are the entries:
Harpar Hopman
Hara Hopman
Harbour Center
Harvard University
Henry Harbour
Hara Jackson

Screenshot 2020-04-23 at 8 51 03 PM

import * as S from '../styles'


export const interfaceSecondaryDropDown: React.FC = () => (
Copy link

Choose a reason for hiding this comment

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

Parsing error: Unexpected token :

@@ -0,0 +1,9 @@
export type SecondaryDropDownProps = {
Copy link

Choose a reason for hiding this comment

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

Parsing error: Unexpected token type

"name": "Hara Jackson"
}

]
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 0 spaces but found 2 indent
Missing semicolon semi

{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Hara Jackson"
}
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent
Missing trailing comma comma-dangle

},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Hara Jackson"
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing trailing comma comma-dangle
Strings must use singlequote quotes
Unnecessarily quoted property 'name' found quote-props

{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harvard University"
},
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harvard University"
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing trailing comma comma-dangle
Strings must use singlequote quotes
Unnecessarily quoted property 'name' found quote-props

"name": "Harbour Center"
},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Strings must use singlequote quotes
Unnecessarily quoted property 'avatarLink' found quote-props

"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harbour Center"
},
{
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harbour Center"
},
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harbour Center"
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing trailing comma comma-dangle
Strings must use singlequote quotes
Unnecessarily quoted property 'name' found quote-props

"name": "Hara Hopman"
},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Strings must use singlequote quotes
Unnecessarily quoted property 'avatarLink' found quote-props

"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Hara Hopman"
},
{
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Hara Hopman"
},
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

},
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Hara Hopman"
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing trailing comma comma-dangle
Strings must use singlequote quotes
Unnecessarily quoted property 'name' found quote-props

"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harpar Hopman"
},
{
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harpar Hopman"
},
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

export const Details = [
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
"name": "Harpar Hopman"
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing trailing comma comma-dangle
Strings must use singlequote quotes
Unnecessarily quoted property 'name' found quote-props


export const Details = [
{
"avatarLink": "https://storage.googleapis.com/ignitus_assets/ig-avatars/eugene.png",
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Strings must use singlequote quotes
Unnecessarily quoted property 'avatarLink' found quote-props

@@ -0,0 +1,29 @@

export const Details = [
{
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 4 indent

@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #808 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##            develop       #808   +/-   ##
===========================================
  Coverage   2.84091%   2.84091%           
===========================================
  Files             5          5           
  Lines           176        176           
  Branches         48         48           
===========================================
  Hits              5          5           
  Misses          171        171           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d81224...c1a272f. Read the comment docs.

import { Details } from '../constants';


export default function SecondaryDropDown() {
Copy link
Member

Choose a reason for hiding this comment

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

To be consistent to code structure

Suggested change
export default function SecondaryDropDown() {
export const SecondaryDropDown: React.FC = () => {

Change accordingly in Components/index.ts

Also make an entry in ignitus-Shared/index.ts, for easy access.

Copy link
Contributor Author

@meghanacosmos meghanacosmos Apr 23, 2020

Choose a reason for hiding this comment

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

Sure!Thank you.

@@ -32,6 +32,7 @@ export {DefaultMultiMediaInput} from './ignitus-DesignSystem/ignitus-Atoms/ignit

export {DefaultSearchInput} from './ignitus-DesignSystem/ignitus-Atoms/ignitus-defaultSearchInput/Components';

export {SecondaryDropDown} from './ignitus-DesignSystem/ignitus-Atoms/ignitus-secondaryDropDown/Components/secondaryDropDown';
Copy link

Choose a reason for hiding this comment

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

A space is required after '{' object-curly-spacing
A space is required before '}' object-curly-spacing

import {Details} from '../constants';


export const SecondaryDropDown: React.FC = () => {
Copy link

Choose a reason for hiding this comment

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

Parsing error: Unexpected token :

@@ -93,6 +95,10 @@ export const UserInterfaceBookRoutes: React.FunctionComponent = () => (
path="/interface/defaultDropdown"
component={interfaceDropDown}
/>
<Route
path="/interface/secondaryDropdown"
Copy link
Member

Choose a reason for hiding this comment

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

I cannot see your dropdown on this route, so can you please add it. Also, try to add the information on how can we view your new PR, like mentioning the route in this case.

Copy link
Member

@divyanshu-rawat divyanshu-rawat left a comment

Choose a reason for hiding this comment

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

Left some feedback, though I resolved it myself but still left will be helpful foe you to enhance your skills. Overall great work 🎯 I told you , you can do ti! 🏆

setShowOptions(true);
};
React.useEffect(() => {
const results = [] = Details.filter(person =>
Copy link
Member

Choose a reason for hiding this comment

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

Did you checked the functionality of dropdown is affected after you added adding [] = , nothing is shown in dropdown the problem is that [] = Details.filter() is equivalent to [] || Details.filter() , so it implies either assign one of the value to array and every-time it assigns it [], so dropdown was always empty, but I fixed it by removing [] =

padding-left:0.25rem;
color: ${C.IgnitusBlue};
font-weight: ${F.SemiBold};
:: placeholder{
Copy link
Member

Choose a reason for hiding this comment

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

use


 &::placeholder {
    color: ${C.IgnitusBlue};
    font-weight: ${F.SemiBold};
  }

export const Avatar = styled.img`
border-radius: 50%;
width : 2.5rem;
margin: 0 1rem 0 1rem;
Copy link
Member

Choose a reason for hiding this comment

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

use 0 1rem;

@@ -38,13 +36,13 @@ export default function SecondaryDropDown() {
{showOptions &&
<S.OptionsContainer>
{searchResults.map((person, index) =>
Copy link
Member

Choose a reason for hiding this comment

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

use de -structure operator to extract { name, avatarLink } in map.

          {searchResults.map(({ name, avatarLink }, index) => (
            <S.CardWrapper key={`${name}`}>
              <S.Avatar src={avatar} />
              <S.NameWrapper>
                <H.Heading5>{name}</H.Heading5>
              </S.NameWrapper>
            </S.CardWrapper>
          ))}

@divyanshu-rawat divyanshu-rawat merged commit 4577a28 into Ignitus:develop Apr 29, 2020
@divyanshu-rawat divyanshu-rawat mentioned this pull request Apr 29, 2020
@meghanacosmos
Copy link
Contributor Author

Left some feedback, though I resolved it myself but still left will be helpful foe you to enhance your skills. Overall great work 🎯 I told you , you can do ti! 🏆

Sure, I'll go through the reviews. Thank you so much

@meghanacosmos meghanacosmos deleted the DropDownSec branch April 30, 2020 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants