Commit 368c47c5 authored by Hugo Lefeuvre's avatar Hugo Lefeuvre Committed by Sébastien Blin

chatview: major cleanup

Despite of the recent optimization commits, the chatview is still
affected by a significant number of performance issues. This commit
tries to address some of them:

* Use getElementById instead of querySelector, when possible.
* Use getElementByClassName instead of querySelector, when possible.

(querySelector is in average 2x slower than getElementById or
getElementByClassName!)

In this commit we also address the following bugs:

* printHistoryPart: it doesn't make any sense to call
  addOrUpdateMessage if we are at the end of the buffer.
* the bottom padding of the body isn't updated with the size of
  the #message textarea, leading to an annoying overlap when writing
  large messages.
* selection highlighting in the chatview is crappy. The user should
  not be able to highlight the background because it doesn't make
  any sense. Only meaningful elements should be highlightable.
* The first time a message fails to be sent, the cross 'X' icon
  is displayed with a very bad looking shift to the right. This is
  because the 'sending' icon is still displayed, with visibility
  hidden.
* Many variables were not declared / using some broken, ancient
  JavaScript Sith magic we don't want to mess with. In order to make
  sure we are not going to rely on such mechanisms anymore, add
  "use strict" stanza.

In addition to that we also fix a _very_ large number of style issues
(semicolons, indentation, comments) and define a clear style policy
for the JS code.

A code audit revealed several small bugs which we are not going to
address in this commit. Add FIXME comments.

Also, remove some useless debug warnings from the GTK side chatview
code.

Change-Id: If6b605868ba6b0b9623ae01c5293064211b58327
Reviewed-by: Sébastien Blin's avatarSebastien Blin <sebastien.blin@savoirfairelinux.com>
parent 547dd535
......@@ -532,7 +532,6 @@ chat_view_update_temporary(ChatView* self, bool showAddButton, bool showInvitati
g_return_if_fail(IS_CHAT_VIEW(self));
auto priv = CHAT_VIEW_GET_PRIVATE(self);
g_debug("chat_view_update_temporary(%s, %s)", showAddButton ? "true":"false", showInvitation ? "true":"false");
update_chatview_frame(self);
}
......
{
"env": {
"browser": true
},
"plugins": ["html"],
"extends": "eslint:recommended",
"parserOptions": {
"ecmaVersion": 6
},
"rules": {
"indent": [
"error",
4
],
"linebreak-style": [
"error",
"unix"
],
"quotes": [
"error",
"double"
],
"semi": [
"error",
"never"
],
"no-inner-declarations": [
0
]
}
}
# README - chatview
The chatview runs under a WebKit GTK view. It is written using web technologies
(HTML5/CSS3/JS) and is responsible for displaying everything that deals with the
navbar, the messages, and the message bar.
## Contributing - syntax
We have a set of ESLint rules that define clear syntax rules (web/.eslintrc.json).
You will need the following tools:
- ESLint (The pluggable linting utility for JavaScript and JSX)
https://eslint.org/
- ESLint HTML plugin (eslint-plugin-html)
https://www.npmjs.com/package/eslint-plugin-html
Before pushing a patch, make sure that it passes ESLint:
$ eslint chatview.html
Most trivial issues can be fixed using
$ eslint chatview.html --fix
We will not accept patches introducing non-ESLint-compliant code.
## WebKit GTK
Everything runs under WebKit GTK, that is if you need to write browser specific
code, you will only need to support WebKit (CSS -webkit- prefix).
Do not use querySelector if getElementById or getElementByClassName can be used
instead. querySelector doesn't always make the code easier and has very bad
performances.
......@@ -12,8 +12,9 @@
--navbar-height: 40px;
--navbar-padding-top: 8px;
--navbar-padding-bottom: var(--navbar-padding-top);
--navbar-size: calc(var(--navbar-height) + var(--navbar-padding-top) + var(--navbar-padding-bottom));
--messagebar-size: 57px; /* FIXME clean this up */
/* message bar properties */
--textarea-max-height: 150px;
/* button properties */
--action-icon-color: var(--ring-dark-blue);
......@@ -54,11 +55,17 @@
/** Body */
body {
--navbar-size: calc(var(--navbar-height) + var(--navbar-padding-top) + var(--navbar-padding-bottom));
--messagebar-size: 57px; /* FIXME clean this up */
margin: 0;
overflow: hidden;
background-color: var(--bg-color);
padding-top: var(--navbar-size);
padding-bottom: var(--messagebar-size);
/* disable selection highlight because it looks very bad */
-webkit-user-select: none;
}
/** Navbar */
......@@ -86,9 +93,6 @@ body {
/* hairline */
border-bottom: var(--hairline-thickness) solid var(--hairline-color);
/* disable selection highlight because it looks very bad */
-webkit-user-select: none;
}
#navbar.hiddenState {
......@@ -134,7 +138,7 @@ body {
height: 100%;
font-family: emoji;
/* enable selection (it is globally disabled in the navbar) */
/* enable selection (it is globally disabled in the body) */
-webkit-user-select: auto;
}
......@@ -293,14 +297,18 @@ body {
}
#message {
font-family: emoji;
flex: 1;
background-color: var(--bg-color);
border: 0;
overflow-y: scroll;
color: black;
max-height: 150px;
max-height: var(--textarea-max-height);
margin-right: 10px;
resize: none;
/* enable selection (it is globally disabled in the body) */
-webkit-user-select: auto;
}
#message:focus,
......@@ -378,6 +386,9 @@ a:hover {
justify-content: flex-start;
align-items: top;
overflow: hidden;
/* enable selection (it is globally disabled in the body) */
-webkit-user-select: auto;
}
.message_in {
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment