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

Swathi Fix resources dot centering on Tasks pages #2789

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

Lemonnycodes
Copy link
Contributor

@Lemonnycodes Lemonnycodes commented Oct 16, 2024

Description

This PR addresses the issue of misaligned initials inside resource dots and improves the layout when there are multiple resources in the task table. The initials inside the resource circles are now correctly centered both vertically and horizontally.

Related PRS (if any):

This frontend PR is related to the development backend PR.

Main changes explained:

  • Updated the CSS file to center the initials in the resource dots using display: flex, justify-content: center, and align-items: center.
  • Applied text-align: center and vertical-align: middle to table cells to align text and icons centrally.

How to test:

  1. check into current branch
  2. do npm install and ... to run this PR locally
  3. Clear site data/cache
  4. log as admin user
  5. go to Other Links>Projects>Click WBS Icon>Choose WBS>Edit
  6. Verify that the initials in resource circles are properly centered.
  7. Add multiple resources (5 or more) to a task and verify that they display correctly without overlap and can be horizontally scrolled if necessary.
  8. Verify this feature works in dark mode

Screenshots or videos of changes:

Before :
image
After :
full view
10+ resources
image

Copy link

netlify bot commented Oct 16, 2024

Deploy Preview for highestgoodnetwork-dev ready!

Name Link
🔨 Latest commit 2efd00b
🔍 Latest deploy log https://app.netlify.com/sites/highestgoodnetwork-dev/deploys/6740249b62978e00083e5f14
😎 Deploy Preview https://deploy-preview-2789--highestgoodnetwork-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@lucy-xfl lucy-xfl self-requested a review October 16, 2024 08:26
lucy-xfl
lucy-xfl previously approved these changes Oct 16, 2024
Copy link

@lucy-xfl lucy-xfl left a comment

Choose a reason for hiding this comment

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

flex works well in dark and light mode.
image

Copy link

@nikhilpittala16 nikhilpittala16 left a comment

Choose a reason for hiding this comment

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

I have tested the PR and everything works as expected.

PR.vid.64.mp4

SharadhaS24
SharadhaS24 previously approved these changes Oct 18, 2024
Copy link

@SharadhaS24 SharadhaS24 left a comment

Choose a reason for hiding this comment

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

Validated the addition of multiple resource types (1, 5, or more) in both light and dark modes, ensuring consistent performance and correct functionality. Its centered and aligned properly.

image image image

@Anoushka21 Anoushka21 self-requested a review October 18, 2024 08:09
@Anoushka21
Copy link

Tested in both modes. Aligned and working well.

PR.2789.mp4

@pallavithorat
Copy link

I have verified the PR by ensuring that the initials in the resource circles are properly centered, and multiple resources display correctly with horizontal scrolling, without overlap. The feature also works seamlessly in dark mode.
PR#2789

NK3101
NK3101 previously approved these changes Oct 19, 2024
Copy link

@NK3101 NK3101 left a comment

Choose a reason for hiding this comment

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

Tested the PR in both light and dark modes. Working as expected.

#2789 #2789-2

shashankhv
shashankhv previously approved these changes Oct 19, 2024
Copy link

@shashankhv shashankhv left a comment

Choose a reason for hiding this comment

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

Verified initials centering, resource display, and tested in dark mode. Works as expected.
swathi-PR.webm

Copy link

@CodewithKoushica CodewithKoushica left a comment

Choose a reason for hiding this comment

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

The changes in this PR work well, and the initials in the resource dots are correctly centered. However, I noticed that the UI makes a noticeable jump when expanding and collapsing the resource section. It would improve the user experience if this transition could be made smoother, possibly by adding a CSS transition effect. Other than that, the fix looks great!

Screen.Recording.2024-10-23.at.5.41.35.PM.mov

@Lemonnycodes Lemonnycodes added the High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible label Oct 25, 2024
nikhilgiri0226
nikhilgiri0226 previously approved these changes Oct 26, 2024
Copy link
Contributor

@nikhilgiri0226 nikhilgiri0226 left a comment

Choose a reason for hiding this comment

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

I have tested the PR. Everything works as expected. The resources icons with initials were aligned properly inside the column horizontally. I have tested it in different screen sizes.

HGN.APP.-.Google.Chrome.2024-10-25.22-47-37.mp4

zhiminV
zhiminV previously approved these changes Oct 26, 2024
Copy link

@zhiminV zhiminV left a comment

Choose a reason for hiding this comment

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

I have tested that the initials in resource circles are properly centered.
and 5 or more resources display correctly without overlap and can be horizontally. And I also verify this feature works in dark mode.
Screenshot 2024-10-25 at 11 25 35 PM
Screenshot 2024-10-25 at 11 20 19 PM

Screenshot 2024-10-25 at 11 26 16 PM

@Anoushka21 Anoushka21 self-requested a review October 26, 2024 09:33
Anoushka21
Anoushka21 previously approved these changes Oct 26, 2024
varun-elango
varun-elango previously approved these changes Oct 26, 2024
Copy link

@varun-elango varun-elango left a comment

Choose a reason for hiding this comment

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

The code changes look good and the functionality is working as expected. The initials are properly centered and multiple resources are displayed correctly without any overlap. Verified in dark mode as well.
PR2789_1
PR2789_2
PR2789_3

Copy link

@bhavya17prakash bhavya17prakash left a comment

Choose a reason for hiding this comment

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

Working as intended in dark mode as well
PR 2789

Rupa2400
Rupa2400 previously approved these changes Oct 26, 2024
Copy link

@Rupa2400 Rupa2400 left a comment

Choose a reason for hiding this comment

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

Tested the PR in both light and dark modes. The PR is working as expected.

PR_2789_1 PR_2789_2 PR_2789_3

}
.table th {
text-align: center; /* Centers the text horizontally */
vertical-align: middle; /* Centers the text vertically */

Choose a reason for hiding this comment

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

Comments for CSS classes are not required. they are self explanatory

@sheetalmangate
Copy link

CSS changes working as per requirement stated. UI changes looks good, just having small suggestion

  1. comments for CSS classes are not required they are self explanatory.

Approving this PR

PR2789_testing

sheetalmangate
sheetalmangate approved these changes Oct 26, 2024
sheetalmangate
sheetalmangate previously approved these changes Oct 26, 2024
samarth9201
samarth9201 previously approved these changes Oct 30, 2024
Copy link
Contributor

@samarth9201 samarth9201 left a comment

Choose a reason for hiding this comment

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

Reviewed the PR Working as expected.
image

@anirudhsk2107 anirudhsk2107 self-requested a review October 30, 2024 02:40
anirudhsk2107
anirudhsk2107 previously approved these changes Oct 30, 2024
Copy link

@anirudhsk2107 anirudhsk2107 left a comment

Choose a reason for hiding this comment

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

Changes are working in light mode and dark mode

Screen Shot 2024-10-29 at 10 36 55 PM
Screen Shot 2024-10-29 at 10 36 47 PM

Copy link

@Rui-Liu-github Rui-Liu-github left a comment

Choose a reason for hiding this comment

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

I reviewed PR #2789 by pulling the latest version from Swathi. The fixed CSS styling works as expected, with the resources dot positioned at the center.

image
image

SupriyaSudini
SupriyaSudini previously approved these changes Nov 1, 2024
Copy link

@SupriyaSudini SupriyaSudini left a comment

Choose a reason for hiding this comment

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

I have verified that the initials in the resource circles are properly centered and tested adding multiple resources (5 or more) to a task to ensure they display correctly without overlap, with horizontal scrolling available when needed.

image

image

image

Copy link

@AshritaCherlapally AshritaCherlapally left a comment

Choose a reason for hiding this comment

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

This code is working as expected, when creating task with adding resources, the resources are aligned on center as expected

PR.2789.mov

audreydieuanh
audreydieuanh previously approved these changes Nov 3, 2024
Copy link

@audreydieuanh audreydieuanh left a comment

Choose a reason for hiding this comment

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

It works correctly both in light and dark mode and with 5+ resources.
Screenshot 2024-11-02 at 9 17 55 PM
Screenshot 2024-11-02 at 9 18 03 PM

Copy link

@hqzhu0913 hqzhu0913 left a comment

Choose a reason for hiding this comment

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

The resources tags center as expected.
But I find a problem with limited resources selections. I am not sure if I didn't test it correctly. But these are not problem of your UI change. Comment me if you see the problem as well.
Screenshot 2024-11-05 at 1 31 37 PM

Screenshot 2024-11-05 at 1 31 05 PM

@humera314
Copy link

I ran the tests on this feature, and it’s working perfectly across all specified conditions, including dark mode.
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.