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

Add settings to shortcuts #416

Merged
merged 34 commits into from
Mar 9, 2024

Conversation

NeverAgain11
Copy link
Collaborator

@tisfeng
Copy link
Owner

tisfeng commented Feb 21, 2024

The highlighted state of this settings button looks a bit strange compared to the Eudic button next to it, and after clicking on "Go to Settings", this highlighted state does not disappear.

image

Copy link
Collaborator

@Jerry23011 Jerry23011 left a comment

Choose a reason for hiding this comment

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

Looks awesome, I just got two issues:

  1. The colour pattern of the switch button is in default, so it is blacker than the pin icon on the right, can we customize it to match the colour of the pin icon?
  2. Do we want to add an option in settings to allow users disable the button or keep it there as an option

Copy link
Collaborator

@AkaShark AkaShark left a comment

Choose a reason for hiding this comment

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

CleanShot 2024-02-22 at 14 07 15@2x
This settings button looks like bigger than other quick link button. May be you can use resizeImage make it better

   EZOpenLinkButton *button = [[EZOpenLinkButton alloc] init];
    NSImage *image = [NSImage imageWithSystemSymbolName:@"switch.2" accessibilityDescription:nil];
    image = [image resizeToSize:CGSizeMake(16, 16)];
    NSColor *tintColor = [NSColor mm_colorWithHexString:@"#C0C1C4"];
    image = [image imageWithTintColor:tintColor];
    button.image = image;

CleanShot 2024-02-22 at 14 34 49@2x

@NeverAgain11
Copy link
Collaborator Author

Fixed problem with setting button.
@Jerry23011 @AkaShark @tisfeng

@Jerry23011
Copy link
Collaborator

The color of the Settings button doesn't seem to match the color of the Google button next to it, what do you think about changing the TintColor to light blue? Or another suitable color.

I would personally prefer to keep it grey. Since blue represents selected in the pin feature, it could be confusing to make the icon blue.

Also, for users who don't enable the other icons, the grey one looks more concise as it fits the accent colour of the app.
截屏2024-02-26 21 49 47

@tisfeng
Copy link
Owner

tisfeng commented Feb 27, 2024

Sounds reasonable, what do the rest of you think about this?

@phlpsong
Copy link
Collaborator

I think grey is better.

@AkaShark
Copy link
Collaborator

Agreed it!

@tisfeng
Copy link
Owner

tisfeng commented Feb 29, 2024

Ok, let's keep it grey.

…hortcuts

# Conflicts:
#	Easydict/Feature/Configuration/Configuration.swift
#	Easydict/Feature/Utility/Swift/Notification/Notification+Name.swift
…hortcuts

# Conflicts:
#	Easydict/Feature/Configuration/Configuration.swift
@NeverAgain11 NeverAgain11 requested a review from tisfeng March 5, 2024 12:09
tisfeng
tisfeng previously approved these changes Mar 6, 2024
@tisfeng
Copy link
Owner

tisfeng commented Mar 6, 2024

Looks good.

@tisfeng
Copy link
Owner

tisfeng commented Mar 8, 2024

Please continue to review.

@tisfeng tisfeng merged commit 1c9c88f into tisfeng:dev Mar 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants