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

Jump to bottom

Tool tip for alert words. #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Tool tip for alert words. #35

wants to merge 1 commit into from

Conversation

chdinesh1089
Copy link
Collaborator

No description provided.

@chdinesh1089 chdinesh1089 force-pushed the cross branch 2 times, most recently from c08879d to 7723515 Compare October 5, 2020 09:56
@chdinesh1089 chdinesh1089 changed the title Fix #34 chips on modified -- start timers only if extension running Oct 5, 2020
@chdinesh1089 chdinesh1089 requested a review from Arun-42 October 5, 2020 10:00
@chdinesh1089 chdinesh1089 mentioned this pull request Oct 5, 2020
Open
@chdinesh1089
Copy link
Collaborator Author

The current UI for the last commit is like this. Looks fine, do we need to make any changes?
image

@Arun-42
Copy link
Collaborator

Arun-42 commented Oct 6, 2020

Added footer. Should hyperlink github, and replace the icon with a high res one.

chdinesh1089
chdinesh1089 commented Oct 6, 2020
View reviewed changes
src/background.js Outdated
@@ -9,6 +9,7 @@ function notify(word) {
type: "basic",
title: "AutoMeet",
message: `The word '${word}' has appeared!`,
iconUrl: "http://www.google.com/favicon.ico",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this accidental?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed it with a force push

@chdinesh1089
Copy link
Collaborator Author

just pushed a commit to add right and wrong URLs indication. It looks like this now:
image

It'd look better if we reduce the margin below the error image(the svg) and add a little padding below Uh-oh,....... I couldn't figure out what was causing that huge padding below the image. Seems like something's wrong with the image's dimensions. @Arun-42 can you look into it?

@chdinesh1089 chdinesh1089 force-pushed the cross branch 2 times, most recently from 82a2044 to 4bbf470 Compare October 7, 2020 05:37
@chdinesh1089
Copy link
Collaborator Author

nvm, I fixed that with a negative margin. It looks like this now:
image

How does it look? if it seems alright, review and merge it.

chdinesh1089
chdinesh1089 commented Oct 7, 2020
View reviewed changes
src/index.css Outdated
Comment on lines 209 to 211
#heart{

}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will this do anything?

@Arun-42
Copy link
Collaborator

Arun-42 commented Oct 7, 2020

@chdinesh1089 is the notification on chrome fixed?

@chdinesh1089
Copy link
Collaborator Author

Merged other commits for the release. Only the tool tip thing is left.

@chdinesh1089 chdinesh1089 changed the title chips on modified -- start timers only if extension running Tool tip for alert words. Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arun-42 Arun-42 Awaiting requested review from Arun-42

@lokesh576 lokesh576 Awaiting requested review from lokesh576

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chdinesh1089 @Arun-42