CHECKOUT-1967 Resolve Accessibility Issues #1061
Conversation
icatalina
force-pushed
the
CHECKOUT-1967
branch
from
b3864d1
to
651b1a2
Compare
assets/scss/optimized-checkout.scss
Outdated
| // Styles form fields. | ||
| // ----------------------------------------------------------------------------- | ||
|
|
||
| .optimizedCheckout-form-input { | ||
| $optmizedCheckout-inputBoxShadow: inset 0 1px 1px stencilColor("optimizedCheckout-formField-shadowColor"); |
There was a problem hiding this comment.
You have a spelling mistake here: optmizedCheckout -> optimizedCheckout :)
There was a problem hiding this comment.
Also, I think we have a specific way of naming SASS variables (i.e.: https://github.com/bigcommerce/sass-style-guide#variables). Do you want to follow that?
|
The schema file is crazy big right now in Cornertone, and ThemeEditor was not really designed to support so many fields from the schema. This is going to slow down ThemeEditor. I'm not opposing this change, just something to think about. If you have the chance, please take a look a TE to see how we can improve performance. |
|
@mcampa very good point. I agree we can look into improving the performance of TE separately. :) |
icatalina
force-pushed
the
CHECKOUT-1967
branch
3 times, most recently
from
ca0ce8b
to
41baea4
Compare
|
I tackled those issues... please have a look 馃檪 |
icatalina
force-pushed
the
CHECKOUT-1967
branch
from
41baea4
to
6436004
Compare
icatalina
changed the title
|
can we get some screenshots & a QE sign off since it has lots of changes to the schema. |
|
Hey @junedkazi, thanks for checking. We already reached out to Jully, hoping that one of the QEs / designers from Stencil team can help verify the changes in the upcoming sprint. There's a separate ticket tracking the QA work (STENCIL-3751). :) |
|
@icatalina - could you rebase into one commit and add a changelog entry? |
|
@mjschock For sure... |
|
I let it like this because it is easier to review... |
icatalina
force-pushed
the
CHECKOUT-1967
branch
2 times, most recently
from
ec0b222
to
f715198
Compare
|
Hi @mjschock!, All squashed and change log added. I've kept a copy not squashed just in case it makes it easier to review: https://github.com/icatalina/cornerstone/tree/CHECKOUT-1967-unsquash Could you please have a look and confirm you're happy with these changes? Kind Regards, |
icatalina
force-pushed
the
CHECKOUT-1967
branch
from
f715198
to
c9bab03
Compare
|
Some screenshots:
|
schema.json
Outdated
| @@ -1,2498 +1,2626 @@ | |||
| [ | |||
There was a problem hiding this comment.
@icatalina - is there a way to get the changes in here without causing the entire schema to differ?
There was a problem hiding this comment.
@icatalina - i have a question about the schema changes and there's a conflict with the CHANGELOG. besides that, these changes LGTM.
icatalina
force-pushed
the
CHECKOUT-1967
branch
from
c9bab03
to
14da99a
Compare
icatalina
force-pushed
the
CHECKOUT-1967
branch
from
14da99a
to
8b483ed
Compare
|
I tackled @mjschock concerns... I'll raise a different PR for changing the |
|
I think it makes sense to squash commits that cannot be checked out individually (i.e.: WIP commits). If each commit is an incremental change that is descriptive and comprehensible, I don't think it's strictly necessary to squash them into one.
Having said that, sometimes, it's actually easier to review the changes or check the history if the commits are squashed into one (or fewer ones). In that case, yes, we should squash the commits. So I think it really depends on the situation. We shouldn't squash the commits just because we need to follow a rule. Anyway, just my two cents 馃槃 |
What?
Adding some missing customisation options for Optimized Checkout so merchants can fully tailor the shopper experience and be fully accessible.
Tickets / Documentation
Screenshots (if appropriate)
On their way 馃槉