Skip to content
This repository was archived by the owner on Apr 4, 2024. It is now read-only.

Modernize large parts of the UI#14

Open
haveyaseen wants to merge 1 commit intoFoldingAtHome:masterfrom
haveyaseen:modernize
Open

Modernize large parts of the UI#14
haveyaseen wants to merge 1 commit intoFoldingAtHome:masterfrom
haveyaseen:modernize

Conversation

@haveyaseen
Copy link
Copy Markdown

@haveyaseen haveyaseen commented Mar 18, 2020

Hi, thought I'd put some work into this interface. Please excuse the huge size of this PR.

Unfortunately I couldn't yet figure out how to get the FAHClient to return the current index.html when visiting wrapper.html so I wasn't able to test the changes using live data. Would appreciate some hints on how to do that if it's even possible.

  • update jQuery to v3.4.1
  • update jQuery UI and theme to v1.12.1
  • remove ancient jQuery Selectbox plugin
  • self-host Oswald font (and all libraries)
  • update normalize.css to v8.0.1
  • import Skeleton CSS library
  • use Skeleton's grid layout for initial responsive layout
  • communicate clickable areas as such using pointer cursor
  • homogenize styling of dynamic values for visual consistency
  • use logo file as background image instead of embedding it
  • format main.js using Prettier

* update jQuery to v3.4.1
* update jQuery UI and theme to v1.12.1
* remove ancient jQuery Selectbox plugin
* self-host Oswald font
* update normalize.css to v8.0.1
* import Skeleton CSS library
* use Skeleton's grid layout for initial responsive layout
* communicate clickable areas as such using pointer cursor
* homogenize styling of dynamic values for visual consistency
* use logo file as background image instead of embedding it
* format main.js using Prettier
</div>
<div class="seven columns">
<div id="box-cause-selector">
<div class="panel-top-heading">I support research fighting...</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you explain please why do we need ellipsis here?

</li>
</ul>
</div>
<p>My computer has <span id="box-status-time unknown">unknown time</span> to
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As I see unknown class wasn't hardcoded before. What is the reason for adding it now?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it normal to have this on an id and not on a class ?

even when Web Control is closed.
</p>
<div id="box-project">Loading...</div>
</dl>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems you have missed opening <dl> tag

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it is supposed to close the ul tag line 159

Suggested change
</dl>
</ul>

Copy link
Copy Markdown

@justpetrovych justpetrovych left a comment

Choose a reason for hiding this comment

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

Nice work

@DanFlannel DanFlannel mentioned this pull request Mar 27, 2020
4 tasks
<div class="panel-top-heading">Points earned</div>
<div>
<span id="box-points-counter">Unknown</span>
<a id="box-user-stats-link" href="#">(see stats)</a></span>
Copy link
Copy Markdown

@qk7b qk7b May 23, 2020

Choose a reason for hiding this comment

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

I see no opening tag for the span here

Suggested change
<a id="box-user-stats-link" href="#">(see stats)</a></span>
<span><a id="box-user-stats-link" href="#">(see stats)</a></span>

@qk7b
Copy link
Copy Markdown

qk7b commented May 23, 2020

Great job here !

May I suggest to add the alt attributes on the img tags that are used as buttons ? This will increase the app accessibility.

@tapper82
Copy link
Copy Markdown

@quentin7b Thanks for thinking about us screen reader users. I use NVDA if any testing is needed pleas give me a shout. I cant code but will install it on windows 10 and test with my screenreader.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants