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

reward button vertical align text #693

Merged
merged 1 commit into from
Mar 16, 2019
Merged

reward button vertical align text #693

merged 1 commit into from
Mar 16, 2019

Conversation

1v9
Copy link
Member

@1v9 1v9 commented Mar 16, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes was maked (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs in NexT website have been added / updated (for new features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Issue resolved: N/A

What is the new behavior?

1.gif

How to use?

In NexT _config.yml:

...

Does this PR introduce a breaking change?

  • Yes.
  • No.

@1v9 1v9 added the CSS label Mar 16, 2019
@stevenjoezhang
Copy link
Contributor

Will be fixed in #689.
屏幕快照 2019-03-16 上午11 43 55
No need to add a hardcoded line height, I think

@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

@stevenjoezhang 😂 I found this because I tried to add option for animation of reward.

@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

is ok to remove W&H?

@stevenjoezhang
Copy link
Contributor

You can merge it, and I'll update #689 then
Also see this: iissnan/hexo-theme-next#1856

@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

@stevenjoezhang yes I saw that in roadmap, this is new style
批注 2019-03-16 115155.jpg

@ivan-nginx ivan-nginx changed the title fix reward button text vertical align Fixed reward button vertical align text Mar 16, 2019
@ivan-nginx ivan-nginx added this to the v7.1.0 milestone Mar 16, 2019
@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

New style is nice I think, how we solve this?

@ivan-nginx
Copy link
Member

Will be fixed in #689.
屏幕快照 2019-03-16 上午11 43 55
No need to add a hardcoded line height, I think

You can merge it, and I'll update #689 then
Also see this: iissnan/hexo-theme-next#1856

See what I'm talking about mixed before? About mixed Features & Fixes & Optimizations. This approach more troubles than we think. 🌗

@ivan-nginx
Copy link
Member

ivan-nginx commented Mar 16, 2019

@stevenjoezhang yes I saw that in roadmap, this is new style
批注 2019-03-16 115155.jpg

Maybe a little bigger? + 3-5px. It's a little bit hard to click on donate...


And live demo, if u can.

@stevenjoezhang
Copy link
Contributor

@ivan-nginx Emm.. I hardly agree it's "mixed". When I implement a new feature, I found something is broken by it (reward button looks strange when font-size is larger than 20px), so I fixed it. Just like this animation shows:

animation 2019-03-16 12_09_53

Adding a new feature is not just adding a feature 🌗 Have to handle the bugs caused by this feature

@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

@ivan-nginx @stevenjoezhang For me, both styles are acceptable, but it's indeed a bit crowded if #689 merged. Maybe keep it like before?

@stevenjoezhang
Copy link
Contributor

@1v9 Maybe you can fix this: iissnan/hexo-theme-next#687 (comment)
Add an option?

@1v9
Copy link
Member Author

1v9 commented Mar 16, 2019

@stevenjoezhang you mean roll animation? It's done but I'm finding is there a better code, you know I'm not an expert.

@stevenjoezhang
Copy link
Contributor

@1v9 Don't worry, take your time. There is a lot of feature requests about reward button, like this: iissnan/hexo-theme-next#687 (comment) 😂

@ivan-nginx
Copy link
Member

Add an option?

Yes, add an option and by default don't shake label under the cursor.

@1v9 1v9 merged commit 45d6780 into theme-next:master Mar 16, 2019
@1v9 1v9 deleted the reward branch March 16, 2019 04:47
@stevenjoezhang
Copy link
Contributor

And this issue: #596. Maybe we need to refactor this function.

@ivan-nginx
Copy link
Member

Dynamic unlimited additions?

@stevenjoezhang
Copy link
Contributor

@ivan-nginx
Copy link
Member

@stevenjoezhang I mean: they repeat same code with just different names. I want to create something like enumeration of arrays to provide any unlimited pictures and names with donations. For example, see menu & submenu(s) implementation with icons to them.

@stevenjoezhang
Copy link
Contributor

@ivan-nginx Yes, I understand what you mean.

@liolok liolok changed the title Fixed reward button vertical align text reward button vertical align text Mar 31, 2019
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants