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.

Sign up for GitHub

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

Fix:Sidebar icons on minimized/closed position are not linked to rele… #264

Closed
wants to merge 3 commits into from
Closed

Fix:Sidebar icons on minimized/closed position are not linked to rele… #264

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3281074
Bug:Sidebar icons on minimized/closed position are not linked to rele…
arkumari2000 Jun 29, 2021
bf52bd7
Merge branch 'develop' of https://github.com/anitab-org/bridge-in-tec…
arkumari2000 Jul 7, 2021
4e4c854
impleented conditional rendering
arkumari2000 Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 40 additions & 31 deletions src/sidebar/Sidebar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { AuthContext } from "../AuthContext";
import "./sidebar.css";
import image from "../assets/images/AnitaBLogo.png";

export default function Sidebar(){
export default function Sidebar() {
const { user, isAuth } = useContext(AuthContext);
const [isNotActive, setNotActive] = useState("true");
const [isDropdownActive, setDropdownActive] = useState("false");
Expand All @@ -15,7 +15,7 @@ export default function Sidebar(){
return (
<div className={!isAuth ? "hidden" : ""}>
Copy link
Contributor

Choose a reason for hiding this comment

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

@arkumari2000 I don't think this is the right way to do conditional rendering. Anyone can change the toggle the display:none css property from the developer tools. Ideally, if isAuth is false, then the component should return null. So please don't use this hidden class as a way of conditional rendering a component.
cc: @mtreacy002

Copy link
Member

@mtreacy002 mtreacy002 Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with @Rahulm2310 , @arkumari2000 . Perhaps the reason why you used hidden is because you didn't run the backend servers in parallel as you're supposed to and use this to by pass the isAuth. For this time, we can let it slide since the manual tests shows that it works when I run the backend servers. But for it to be merged to the upstream branch, can you please remove the hidden option as per @Rahulm2310 suggested? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sure, I will do the conditional rendering and not hidden property.

<div className="wrapper">
<nav id="sidebar" className={isNotActive ? "active" : ""}>
<nav id="sidebar" className={isNotActive ? "sidebar-active" : ""}>
<button
type="button"
id="sidebarCollapse"
Expand All @@ -25,77 +25,86 @@ export default function Sidebar(){
<span className={isNotActive ? "" : "hidden"}>{barsIcon}</span>
<span className={isNotActive ? "hidden" : ""}>{crossIcon}</span>
</button>
<div className="sidebar-header">
<img
alt="user_logo"
src={image}
className="rounded-circle usr-image"
height={isNotActive ? "20" : "70"}
width={isNotActive ? "20" : "70"}
></img>
<h3>{user}</h3>
</div>
<div className="sidebar-header">
<img
alt="user_logo"
src={image}
className="rounded-circle usr-image"
height={isNotActive ? "20" : "70"}
width={isNotActive ? "20" : "70"}
></img>
<h3>{user}</h3>
</div>

<ul className ="list-unstyled components">
<ul className="list-unstyled components">
<li className="list-item">
<i className="fas fa-briefcase icon-color"></i>
<Link to="/members">Members</Link>
<Link to="/members" className="sidebar-link">
<i className="fas fa-briefcase icon-color"></i>
<span className="sidebar-link-name">Members</span>
</Link>
</li>
<li className="list-item">
<i className="fas fa-building icon-color"></i>
<Link to="/organizations">Organizations</Link>
<Link to="/organizations" className="sidebar-link">
<i className="fas fa-building icon-color"></i>
<span className="sidebar-link-name">Organizations</span>
</Link>
</li>
<li className="list-item">
<i className="fas fa-user-alt icon-color"></i>
<OverlayTrigger
key="bottom"
placement="bottom"
overlay={
<Tooltip id={`tooltip-$'bottom'`}>{!isDropdownActive ? "Close My Space" : "Open My Space"}</Tooltip>
<Tooltip id={`tooltip-$'bottom'`}>
{!isDropdownActive ? "Close My Space" : "Open My Space"}
</Tooltip>
}
>
<Link
to="/portfolio"
href="#homeSubmenu"
data-toggle="collapse"
aria-expanded="false"
className="dropdown-toggle"
className="dropdown-toggle sidebar-link"
onClick={() => setDropdownActive(!isDropdownActive)}
>
My Space
<i className="fas fa-user-alt icon-color"></i>
<span className="sidebar-link-name">My Space</span>
</Link>
</OverlayTrigger>
<ul
className={
isDropdownActive ? "list-unstyled collapse" : "list-unstyled"
}
id="homeSubmenu"
>
<li className="dropdown-item">
<Link to="/portfolio">Portfolio</Link>
<Link to="/portfolio" className="sidebar-link">Portfolio</Link>
</li>
<li className="dropdown-item">
<Link to="/personal-details">Personal Details</Link>
<Link to="/personal-details" className="sidebar-link">Personal Details</Link>
</li>
<li className="dropdown-item">
<Link to="/additional-info">Additional Info</Link>
<Link to="/additional-info" className="sidebar-link">Additional Info</Link>
</li>
<li className="dropdown-item">
<Link to="/personal-background">Personal Background</Link>
<Link to="/personal-background" className="sidebar-link">Personal Background</Link>
</li>
</ul>
</li>
<li className="list-item">
<i className="fas fa-history icon-color"></i>
<Link to="/request-history">Request History</Link>
<Link to="/request-history" className="sidebar-link">
<i className="fas fa-history icon-color"></i>
<span className="sidebar-link-name">Request History</span>
</Link>
</li>
<li className="list-item">
<i className="fas fa-sitemap icon-color"></i>
<Link to="/organization-profile">My Organization</Link>
<Link to="/organization-profile" className="sidebar-link">
<i className="fas fa-sitemap icon-color"></i>
<span className="sidebar-link-name">My Organization</span>
</Link>
</li>
</ul>
</nav>
</div>
</div>
);
};
}
33 changes: 19 additions & 14 deletions src/sidebar/sidebar.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
margin: 1rem;
text-align: left;
}
#sidebar ul li a{
.sidebar-link{
text-decoration: none;
padding-left: .5rem;
color: #e9e9ea;
}
.sidebar-link-name{
margin-left: .5rem;
color: #e9e9ea;
}
Expand All @@ -31,10 +36,7 @@
padding: 10px;
border-radius: 5px;
background-color: #362F78;
}
#sidebar li a{
text-decoration: none;
padding-left: .5rem;
text-decoration: none;
}
.icon-color{
color: #738a9b;
Expand All @@ -53,25 +55,28 @@
#sidebar .dropdown-item:hover{
background-color: #215cad;
}
.dropdown-item .sidebar-link:hover{
color: #e9e9ea;
}
.usr-image{
border: 2px solid white;
}
.collapse{
display: none;
}
#sidebar.active .sidebar-header h3 {
#sidebar.sidebar-active .sidebar-header h3 {
display: none;
}
#sidebar ul li a {
#sidebar ul li {
text-align: left;
}
#sidebar.active ul li a {
display: none;
}
#sidebar.active ul li i {
#sidebar.sidebar-active ul li a i {
display: block;
}
#sidebar.active .dropdown-toggle::after {
#sidebar.sidebar-active .sidebar-link-name {
display: none;
}
#sidebar.sidebar-active .dropdown-toggle::after {
top: auto;
bottom: 10px;
right: 50%;
Expand All @@ -84,7 +89,7 @@
}

@media (max-width: 768px) {
#sidebar.active {
#sidebar.sidebar-active {
width: 50px;
text-align: center;
margin: 0;
Expand All @@ -95,7 +100,7 @@
#sidebarCollapse{
right: .5rem;
}
#sidebar.active .sidebar-header h3 {
#sidebar.sidebar-active .sidebar-header h3 {
display: none;
}
.dropdown-toggle::after {
Expand Down