-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Store credit admin UI fixes #2426
Conversation
81ba8de
to
449e582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this also include #2425 right?
I just left a comment about i18n keys, the rest is ok for me, thanks!
core/config/locales/en.yml
Outdated
category_id: Credit Type | ||
amount_authorized: Authorized | ||
amount_credited: Credited | ||
amount_used: Used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure these 3 keys are ok, since it could not be clear that they refer to the amount.
Eg.
Used cannot be greater than the credited amount
I don't get what we gain removing the term Amount
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I will change that
build-ci.rb
Outdated
# @return [Boolean] | ||
# the success of the installation | ||
def self.bundle_install | ||
system(*%W[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass array contents as separate arguments.
build-ci.rb
Outdated
# | ||
# @return [Boolean] | ||
def self.bundle_check | ||
system(*%W[bundle check --path=#{VENDOR_BUNDLE}]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass array contents as separate arguments.
ceda8a0
to
65c9f4c
Compare
The current balance already indicates the currency of the store credit tables belonging to a currency group.
Currently it is a untranslated boolean
We have too many borders in our layout. Whitespace, tables and headers are separating content areas already, no need additional borders. That only distracts the eye. Keeps the bottom border for fieldsets that are holding a form. This makes sense as the form buttons sit on top of the bottom border and visually encloses the form field set.
65c9f4c
to
5d4b8e5
Compare
@kennyadsl made the requested changes. Found old unused translation keys and used them for the table headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Builds on top of #2419
Before
After