Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

ui:Changed the position of admin name in profile section #1099

Closed
wants to merge 1 commit into from
Closed

ui:Changed the position of admin name in profile section #1099

wants to merge 1 commit into from

Conversation

yash2189
Copy link
Contributor

@yash2189 yash2189 commented Sep 8, 2020 •
edited
Loading

Description

Change the positioning of the admin name in the profile section
Fixes #1044

Type of Change:

  • User Interface

How Has This Been Tested?

Centered the text using CSS so that it is clearly visible.
Capture

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials

@yash2189
Copy link
Contributor Author

yash2189 commented Sep 8, 2020

@sanchibansal340 why did the build fail?

SanketDG
SanketDG suggested changes Sep 8, 2020
View reviewed changes
Copy link

@SanketDG SanketDG left a comment

Choose a reason for hiding this comment

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

I understand this is easier to implement but there's a reason why a standard like CSS exists 🙂

Please try this using CSS, it's not that hard.

@yash2189
Copy link
Contributor Author

yash2189 commented Sep 8, 2020 •
edited
Loading

Should I bring that text on center?@SanketDG

@yash2189
Copy link
Contributor Author

yash2189 commented Sep 8, 2020 •
edited
Loading

please review my PR #1099 @sanchibansal340 @SanketDG

@yash2189 yash2189 mentioned this pull request Sep 8, 2020
Open
3 tasks
@yash2189
Copy link
Contributor Author

yash2189 commented Sep 9, 2020

@sanchibansal340 @SanketDG can you please review my PR

@sanchibansal340
Copy link

Will do @yash2189

SanketDG
SanketDG reviewed Sep 10, 2020
View reviewed changes
vms/administrator/templates/administrator/profile.html Outdated
@@ -5,7 +5,7 @@
{% block content %}
<div class="spacer"></div>
<h1 class="header">
{{ administrator.first_name }} {{ administrator.last_name }}
<p style = "text-align:center;">{{ administrator.first_name }} {{ administrator.last_name }}</p>
Copy link

Choose a reason for hiding this comment

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

You can just add some CSS to .header instead of adding inline CSS

@yash2189
Copy link
Contributor Author

yash2189 commented Sep 10, 2020 •
edited
Loading

@SanketDG I have done the required changes. Please review the same.

SanketDG
SanketDG approved these changes Sep 10, 2020
View reviewed changes
Copy link

@SanketDG SanketDG left a comment

Choose a reason for hiding this comment

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

LGTM!

@yash2189
Copy link
Contributor Author

@sanchibansal340 is my PR currently being reviewed. I asked this because it's been about 9 days. Please tell me if I need to change anything.

gaurivn
gaurivn approved these changes Sep 20, 2020
View reviewed changes
Copy link
Member

@gaurivn gaurivn left a comment

Choose a reason for hiding this comment

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

Good, thank you

@sanchibansal340 sanchibansal340 added the Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. label Sep 27, 2020
@devkapilbansal
Copy link
Member

@sanchibansal340 , I am not able to re-generate that UI error. Can you please provide me with steps on how to proceed?

@sanchibansal340
Copy link

@devkapilbansal Even I am not able to see the error now. I am not sure. I guess it got fixed.

@devkapilbansal
Copy link
Member

@devkapilbansal Even I am not able to see the error now. I am not sure. I guess it got fixed.

@sanchibansal340 then, is this PR still relevant now??

@sanchibansal340
Copy link

@devkapilbansal I guess not.

@devkapilbansal
Copy link
Member

Okay, then we should close this pull request and the issue linked with it if it's not relevant.

@yash2189
Copy link
Contributor Author

yash2189 commented Oct 16, 2020 •
edited
Loading

@sanchibansal340 will it be counted as a contribution for OSH? The name was appearing at the leftmost corner and I had centered it by adding CSS

@rpattath 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
@yash2189 yash2189 closed this by deleting the head repository Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gaurivn gaurivn gaurivn approved these changes

@SanketDG SanketDG SanketDG approved these changes

Assignees
No one assigned
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change the positioning of the admin name in the profile section
6 participants
@yash2189 @sanchibansal340 @devkapilbansal @SanketDG @gaurivn @rpattath