Skip to content

Status.im theme#5578

Merged
ara4n merged 57 commits intodevelopfrom
matthew/status
Nov 13, 2017
Merged

Status.im theme#5578
ara4n merged 57 commits intodevelopfrom
matthew/status

Conversation

@ara4n
Copy link
Copy Markdown
Member

@ara4n ara4n commented Nov 11, 2017

Land the long-lived Status.im theme branch. Obviously this should ideally be split up into PRs, but unfortunately life is too short atm :/

Twinned with matrix-org/matrix-react-sdk#1605

Stuff to be aware of here:

  • Completely refactors and consolidates the CSS for buttons (using a SCSS mixin rather than an CSS class)
  • Complete rewrites the Tinter to handle tintable themes
  • Might introduce regressions into the default Riot theme where refactoring happened in order to incorporate the Status theme.

Comment thread config.sample.json
"disable_custom_urls": false,
"disable_guests": false,
"disable_login_language_selector": false,
"disable_3pid_login": false,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note to self: These should be pulled into GranularSettings when it lands

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

decided to not make these granular - they are app-level anyways and affect people who aren't logged in, so granularity isn't feasible.

Comment thread docs/skinning thoughts.md
- e.g. if we replace `Login.js` with `StatusLogin.js`, then we should similarly be able to replace `_Login.scss` with `_StatusLogin.scss`.

random thoughts;
- should we be able to change the entire skin at runtime (more like wordpress) - to the extent of replacing entire components?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about color pickers for now? Not like just tint but changing different parts of the Design without needing to change the code

Comment thread src/vector/index.js Outdated
if (!preventRedirect) {
if (/iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream) {
if (confirm(_t("Riot is not supported on mobile web. Install the app?"))) {
window.location = "https://status.im/join-riot.html";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Won't that cause problems for selfhosted Riots?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

thanks for noticing this.
Yes, please don't hijack people

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

heh, looks like this crept in whilst my back was turned. of course it’s not meant to be there; will turn it into a config options. thanks!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ara4n Note a few lines below this is the same line again ;)

@ara4n
Copy link
Copy Markdown
Member Author

ara4n commented Nov 13, 2017

@dbkr LGTMed this at high level OOB, so merging. It'll probably break everything, but y'all should be testing /staging for 0.13.0-rc2 anyway, and I want to give @turt2live a chance to merge it to his GranularSettings tonight (and then merge that too).

@ara4n ara4n merged commit 557fd8f into develop Nov 13, 2017
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