forum.webdiplomacy.net

webDip dev coordination forum / public access todo list
It is currently Thu Nov 23, 2017 5:10 am

All times are UTC




Post new topic Reply to topic  [ 6 posts ] 
Author Message
PostPosted: Wed Dec 28, 2011 9:05 pm 
Offline

Joined: Sat Mar 28, 2009 7:13 am
Posts: 185
A picture is worth a thousand words, so here is two thousand worth...
Attachment:
forum1.png
forum1.png [ 55.69 KiB | Viewed 2691 times ]
and
Attachment:
forum2.png
forum2.png [ 28.6 KiB | Viewed 2691 times ]

Critiques welcome of course!

The 4 lines and 500 characters is the current limits, I just added text making it clear what the limitation was. The error message says "less than 4 lines" but it is really looking for 4 line breaks which is 5 lines. So I changed the error message to say "less than 5 lines" as well as adjusting it to reference the additional message box.

And perhaps instead of "Subject:", "Message:", and "Additional Message:"; the labels should be "Subject:", "Summary:", and "Message:"?

And now I really need to stop playing around and read up some more about git and github.


Top
 Profile  
 
PostPosted: Wed Dec 28, 2011 9:06 pm 
Offline

Joined: Sat Mar 28, 2009 7:13 am
Posts: 185
Hrmm, by the way if I am breaking protocol by posting this here instead of the idea/request forum which you would then move to here if you liked it, please let me know.

Thanks.


Top
 Profile  
 
PostPosted: Fri Dec 30, 2011 4:24 am 
Offline
Site Admin

Joined: Sat Jun 28, 2008 6:24 am
Posts: 892
Very neat idea, and I think it'd be well used. I agree that summary / message would be better wording in this updated interface

I think we should definitely include this, perhaps this could be a good test case for trying a git "pull" since the code isn't likely to do anything weird


Top
 Profile  
 
PostPosted: Fri Dec 30, 2011 7:46 am 
Offline

Joined: Sat Mar 28, 2009 7:13 am
Posts: 185
I've submitted the pull request: https://github.com/kestasjk/webDiplomacy/pull/7

I tried to keep it to just the necessary changes without any extra "cleanup". For example:
Code:
if( !isset($_REQUEST['newmessage']) ) $_REQUEST['newmessage'] = '';
if( !isset($_REQUEST['newsubject']) ) $_REQUEST['newsubject'] = '';
if( !isset($_REQUEST['newmessagereply']) ) $_REQUEST['newmessagereply'] = '';

$new = array('message' => "", 'subject' => "", 'id' => -1);
if( $User->type['User'] AND
    ((isset($_REQUEST['newmessage']) AND ($_REQUEST['newmessage'] != "")) OR
     (isset($_REQUEST['newmessagereply']) AND ($_REQUEST['newmessagereply'] != ""))) )
{
in the lower if statement, why check isset() when just above we ensured they were set?

Code:
      elseif( strlen($new['subject'])>=90 )
      {
         $messageproblem = "Subject is too long, please keep it within 90 characters.";
         $postboxopen = true;
      }
If you do a subject that is exactly 90 wrong, you'll get this error message saying "within 90". If you want to allow 90 characters, then it should check > 90.

And then the code would be easier to follow if we used variables of newsubject, newsummary, and newreply, also changing the reply box to use newreply. But that is just cosmetic and you have to be careful to what you change to what since newmessage becomes newsummary for new messages but newreply for new replies.

I would prefer to make those changes, but probably better to do it separate from the functional changes.


Top
 Profile  
 
PostPosted: Sat Dec 31, 2011 12:36 am 
Offline
Site Admin

Joined: Sat Jun 28, 2008 6:24 am
Posts: 892
From the pull request (I'll try and keep discussion here rather than github):

-----------

Merged the profile.php changes, but now that I've seen it in action I think the summary/message box change might be confusing without some kind of extra guidance. I might add some javascript to make the additional comment area collapsed when first opened, but you can open by selecting it or it'll open when you approach the first-message limit ?

Anyway we'll have to think about that, but the logic behind it looks good and I've tested it etc, so it should only be an interface change left

-----------

Basically just thinking if it can be made any simpler for people writing short messages, or who think the summary is the optional one. The server-side stuff won't need revision though


I'm not sure about those isset() checks, the forum is the oldest code in the system and probably the longest single procedural block, so little things will have collected. :( Feel free to remove redundant code


Top
 Profile  
 
PostPosted: Sat Dec 31, 2011 4:44 am 
Offline

Joined: Sat Mar 28, 2009 7:13 am
Posts: 185
I saw your comments github but didn't have time to respond right away, sorry. In any case...

We could put "(optional)" after the "Message:" label. Or do that and also change the label to "First Reply:" or "Additional Text:" or such.

At first thought I liked the idea of the extra block only showing up after a certain amount of text had been entered in the summary box, but then I thought that many regular forum users may still post a short summary and a separate reply not realizing they could now get to that other box.

Opening it by selecting it might be the best way to go.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 6 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group