openedx / openedx-app-ios

The mobile app for iOS for the Open EdX Platform.
Apache License 2.0
19 stars 13 forks source link

chore: design team feedback to improve app theming capability #365

Closed saeedbashir closed 3 months ago

saeedbashir commented 3 months ago

This PR improves the theme capability of the app by introducing more color options. And also, a custom placeholder is added to all the input fields. By adding the custom placeholder, it will help in meeting the a11y requirements.

Note: The rounded and rectangular corners are configurable, via config files.

Visually, the following changes are made to the openedX theme:

Old Screen New Screen
volodymyr-chekyrta commented 3 months ago

My understanding from yesterday's weekly design meeting is that these changes are still not approved, so they can't be merged. cc @moiz994 @ekangedx @sdaitzman @marcotuts

moiz994 commented 3 months ago

@volodymyr-chekyrta I believe the design feedback was to bring back the subtitle on the sign-in and register screens. The rest are just theme-based changes that will not affect the open edX build.

The new copy of the subtitles is as follows:

Sign-in screen: "Welcome back! Sign in to access your courses."

Register screen: "Create an account to start learning today!"

Let me know what you think.

@marcotuts @sdaitzman @ekangedx what do you guys think of the new subtitles?

sdaitzman commented 3 months ago

Those new subtitles make sense to me. They're more friendly/welcoming/natural vs. the old copy there.

What are the other changes in this PR?

Are there any other changes I'm missing?

saeedbashir commented 3 months ago

Adding a theming option to eliminate border radius on buttons and text fields, with the default staying the same (rounded)

This is already implemented and part of the current codebase.

Adding a custom placeholder to all input fields: is this shown in the screenshots above? Is it to support different builds having different placeholder text for some fields?

@sdaitzman This PR adds support for configuring, input text fields, background color, placeholder color, and input text color. The placeholder texts will remain the same for the open edX community.

I've added this capability the input background colors for open edX are different for light and dark modes, but for edX its same.

saeedbashir commented 3 months ago

My understanding from yesterday's weekly design meeting is that these changes are still not approved, so they can't be merged. cc @moiz994 @ekangedx @sdaitzman @marcotuts

@volodymyr-chekyrta the subtitles are reverted back and updated with latest text. It's ready for review.

saeedbashir commented 3 months ago

My understanding from yesterday's weekly design meeting is that these changes are still not approved, so they can't be merged. cc @moiz994 @ekangedx @sdaitzman @marcotuts

@volodymyr-chekyrta the subtitles are reverted back and updated with latest text. It's ready for review.

volodymyr-chekyrta commented 3 months ago

My understanding from yesterday's weekly design meeting is that these changes are still not approved, so they can't be merged. cc @moiz994 @ekangedx @sdaitzman @marcotuts

@volodymyr-chekyrta the subtitles are reverted back and updated with latest text. It's ready for review.

@saeedbashir thank you! 🚀

volodymyr-chekyrta commented 3 months ago

@saeedbashir, could you please fix the lint commit error before merge?

saeedbashir commented 3 months ago

@saeedbashir, could you please fix the lint commit error before merge?

It's happening because of merge branch message and it's bit hard to reword that message, anyhow it will be fixed in squash while merging the PR.

volodymyr-chekyrta commented 3 months ago

@saeedbashir, could you please fix the lint commit error before merge?

It's happening because of merge branch message and it's bit hard to reword that message, anyhow it will be fixed in squash while merging the PR.

👍