Skip to content
This repository has been archived by the owner on Dec 20, 2022. It is now read-only.

Include more detailed notes and instructions in the attachment upload page #2

Merged
merged 15 commits into from
Sep 13, 2013

Conversation

wallrj
Copy link
Contributor

@wallrj wallrj commented Aug 27, 2013

Adds answers (or links to answers) to some of the most frequent mistakes found in contributed patches directly in the attachment upload page.

I haven't tried building trac-config to test this.

@wallrj
Copy link
Contributor Author

wallrj commented Aug 27, 2013

I got braid working locally so I have been able to test it.

I added the full boostrap.css to get the fluid layout and the alert styles, but unfortunately the reset CSS in that file breaks the lists of checkboxes on the search and ticket report pages.

But it looks like I can just regenerate bootstrap.min.css to include the parts I need using their customiser.

I also made some unrelated changes because I couldn't get pypy installed on my ubuntu virtual machine. Ignore those.

Here's a screenshot.

screenshot from 2013-08-27 22 38 33

@tomprince
Copy link
Contributor

This look nice. Thanks for working on this.

A couple of things:

  1. Please wrap lines of text at sentences, or at natural breaks (e.g. punctuation), rather than at a fixed width.
  2. This doesn't appear to have changes to the associated css files. (Those files are served by the front-end in t-web and are currently versioned there. It would probably make sense to move those here; and have the front-end point at this copy of the files, or have a copy of this repository for those files).
  3. Please remove the extraneous changes here.

…-web and I can't recreate the current customised css anyway. The new content looks reasonably smart without the bootstrap formatting anyway.
@wallrj
Copy link
Contributor Author

wallrj commented Sep 4, 2013

Tom,

  1. Done.
  2. The warning boxes in the original screenshot relied on some extra CSS from the full bootstrap.css
    1. That isn't a final solution though because it interferes with
      the existing styles on the site (probably because of its reset
      rules)
    2. I tried recreating current bootstrap.min.css and adding just the
      new warnings and fluid layout rules using the bootstrap
      customiser, but that's only available for the latest version of
      bootstrap...and generates CSS which looks significantly
      different to what we've got.
    3. It's further complicated because I think we've also added some
      modifications to the end of that file.
    4. So for now, I've left the CSS selectors on the new content but
      without changing the CSS file -- so they have no effect. (see
      new screenshot)
    5. We can upgrade bootstrap in a later ticket and include the bits
      I need.
    6. Moving the css and javascript to trac-config would make this
      easier -- but I'll leave that to you.
  3. I tried reverting the changes I made to "start" but github still
    shows differences -- which are invisible to me. (doesn't seem to
    be any difference in the indentation.)

screenshot from 2013-09-04 21 05 53

@tomprince tomprince merged commit 295b098 into twisted-infra:master Sep 13, 2013
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.

None yet

2 participants