Opened 7 years ago

Closed 7 years ago

#12327 closed enhancement (fixed)

Improve loading page of Mac App

Reported by: iandrus Owned by: iandrus
Priority: major Milestone: sage-5.2
Component: interfaces Keywords: mac app, mac
Cc: kcrisman Merged in: sage-5.2.beta0
Authors: Ivan Andrus Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11080 Stopgaps:

Description

The current loading page of the Mac App is hideous and not as informative as it could be. Also the link to localhost:8000 will soon become localhost:8080 with the new flask notebook (see #11080). Both of these should be fixed without remorse.

Attachments (1)

trac_12327-loading-page.patch (11.9 KB) - added by iandrus 7 years ago.
Fixed typo

Download all attachments as: .zip

Change History (20)

comment:1 follow-up: Changed 7 years ago by kcrisman

Yes! And tested without remorse!

comment:2 in reply to: ↑ 1 Changed 7 years ago by iandrus

  • Status changed from new to needs_review

Replying to kcrisman:

Yes! And tested without remorse!

Let it be so.

comment:3 Changed 7 years ago by kcrisman

See here for the coffee quote.

comment:4 follow-up: Changed 7 years ago by kcrisman

  • Authors set to Ivan Andrus
  • Dependencies set to #11080
  • Reviewers set to Karl-Dieter Crisman

More or less all looks ok. In line 127 we have

<p> If the server is already runn

but I think HTML will ignore the extra whitespace. I assume the CSS is all pretty nice; is there a reason not to use default sizes for things like h1, h2, h3?

It's taking too long for the dmg to build, so I'll have to finish the review tomorrow morning. Also modulo whether port 8080 actually is the new flask port, but #12310 and other references would seem to cover this if I make the new notebook a dependency. That makes this and #11080 dependencies of each other, but I think that's appropriate.

Unless... what about 84:ticket:11080? That's not addressed here, is it addressed elsewhere? Otherwise you might want to make this still port 8000 so it can get merged and address that where you take care of the other issue.

comment:5 in reply to: ↑ 4 Changed 7 years ago by iandrus

Replying to kcrisman:

More or less all looks ok. In line 127 we have

<p> If the server is already runn

but I think HTML will ignore the extra whitespace. I assume the CSS is all pretty nice; is there a reason not to use default sizes for things like h1, h2, h3?

Yes it should ignore the whitespace. As to sizes of h1, etc. I just modified some css from sagemath.org so that I would get the colors right and they had changed the sizes.

It's taking too long for the dmg to build, so I'll have to finish the review tomorrow morning. Also modulo whether port 8080 actually is the new flask port, but #12310 and other references would seem to cover this if I make the new notebook a dependency. That makes this and #11080 dependencies of each other, but I think that's appropriate.

I think that's probably right.

Unless... what about 84:ticket:11080? That's not addressed here, is it addressed elsewhere? Otherwise you might want to make this still port 8000 so it can get merged and address that where you take care of the other issue.

That's not addressed here, nor elsewhere. When I finally got 5.0.beta1 to build and tested it, it wasn't a problem. I'm going to comment on it over there as well now that you reminded me.

Now that I think about it people might run older versions of Sage and then it would be port 8000 anyway. It would be easy to put 2 links. I'm not sure it's worth it though since they should never have to click the link, and the server be running on a different port anyway since 8000 or 8080 might have been taken by something else (e.g. hg serve). It's trivial to add it, but I don't want to do it if it means you will have to build the dmg again to review it. FWIW you can set SAGE_APP_DMG to no to avoid building the dmg, though it still takes forever to build the application itself.

comment:6 Changed 7 years ago by kcrisman

  • Status changed from needs_review to positive_review

I'm not worried about the port; anyone who knows that much and is intentionally using an older (?!) version of Sage is going to know to use another port.

Ok, then. It looks great! The tips are amusing, though hopefully people won't have to go through all of them, and they don't seem to appear completely randomly.

As to building, my understanding is that SAGE_APP_DMG=no still builds Sage as a tgz, just not a dmg. Anyway.

comment:7 Changed 7 years ago by jason

I guess this line has a typo: "Sometimes that best way to solve find a bug is to explain it to a rubber duck "

Changed 7 years ago by iandrus

Fixed typo

comment:8 Changed 7 years ago by iandrus

Good catch, thanks.

comment:9 follow-up: Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.0 to sage-pending

comment:10 in reply to: ↑ 9 ; follow-up: Changed 7 years ago by kcrisman

Replying to jdemeyer:

milestone changed from sage-5.0 to sage-pending

Is this because it's waiting on #11080? Just clarifying - that would certainly make sense.

comment:11 in reply to: ↑ 10 Changed 7 years ago by jdemeyer

Replying to kcrisman:

Is this because it's waiting on #11080?

and #11874 and #11503.

comment:12 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.1

comment:13 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.1 to sage-pending

comment:14 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.2

comment:15 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.2.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:16 Changed 7 years ago by jdemeyer

  • Merged in sage-5.2.beta0 deleted
  • Milestone changed from sage-5.2 to sage-pending
  • Resolution fixed deleted
  • Status changed from closed to new

Unmerging this from sage-5.2 due to the serious security issue at #13270.

comment:17 Changed 7 years ago by jdemeyer

  • Status changed from new to needs_review

comment:18 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:19 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.2.beta0
  • Milestone changed from sage-pending to sage-5.2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.