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

feat: fixed notification text overlapping #3334

Merged
merged 5 commits into from
Nov 27, 2022

Conversation

VaibhavUpreti
Copy link
Member

Fixes #3333

Describe the changes you have made in this PR -

Minor changes in CSS

Screenshots of the changes (If any) -

Before fix

Fixed screen
Screenshot 2022-11-01 at 5 54 21 PM

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

@coveralls
Copy link

coveralls commented Nov 1, 2022

Coverage Status

Coverage remained the same at 82.034% when pulling 2c191f3 on VaibhavUpreti:notification-overlap into d27a74b on CircuitVerse:master.

@vedant-jain03
Copy link
Member

cc @nitin10s

@vedant-jain03 vedant-jain03 added the platform Related to new UI fixes label Nov 3, 2022
@vedant-jain03
Copy link
Member

vedant-jain03 commented Nov 7, 2022

Screenshot from 2022-11-07 10-43-12

I tested it with the long name!

I am thinking of adding the unseen span at the beginning of the message
It will look something like this:
Screenshot from 2022-11-07 10-51-30

WDYT @VaibhavUpreti @tachyons @nitin10s

@VaibhavUpreti
Copy link
Member Author

Screenshot from 2022-11-07 10-43-12

I tested it with the long name!

I am thinking of adding the unseen span at the beginning of the message It will look something like this: Screenshot from 2022-11-07 10-51-30

WDYT @VaibhavUpreti @tachyons @nitin10s

I think it looks great 👍. This new approach looks good, I like it.

@vedant-jain03
Copy link
Member

@VaibhavUpreti Could you add the suggested UI in #UX in the slack channel?

@VaibhavUpreti
Copy link
Member Author

Screenshot 2022-11-15 at 1 47 47 AM
@vedant-jain03 made final changes after suggestions, kindly approve.

@VaibhavUpreti
Copy link
Member Author

@nitin10s pls review

@vedant-jain03 vedant-jain03 added this to Ready for Review in Main Platform Improvement Nov 16, 2022
@vedant-jain03
Copy link
Member

@VaibhavUpreti I have added a commit, try it out!

@vedant-jain03 vedant-jain03 moved this from Ready for Review to In progress in Main Platform Improvement Nov 16, 2022
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
@VaibhavUpreti
Copy link
Member Author

@VaibhavUpreti I have added a commit, try it out!
Screenshot 2022-11-16 at 11 54 10 PM
This solves our problem 👍

@vedant-jain03
Copy link
Member

@VaibhavUpreti Could you please work on checks, this PR is ready to merge then!

@VaibhavUpreti
Copy link
Member Author

@vedant-jain03 had to write some extra CSS as the notification modal and page were inconsistent ... here is the view after fix.
Screenshot 2022-11-17 at 7 26 21 PM

Screenshot 2022-11-17 at 7 27 10 PM

@codeclimate
Copy link

codeclimate bot commented Nov 17, 2022

An error occurred when fetching issues.

View more on Code Climate.

@tachyons tachyons merged commit 144b697 into CircuitVerse:master Nov 27, 2022
@vedant-jain03 vedant-jain03 moved this from In progress to Completed(Merged) in Main Platform Improvement Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform Related to new UI fixes
Projects
Main Platform Improvement
Completed(Merged)
Development

Successfully merging this pull request may close these issues.

Overlapping Notification and unread notification green circle
4 participants