Adds help icon for choosing new page type #223

Merged

dianawi merged 1 commit from more-helpicons into master

2019-04-09 08:40:18 +00:00

dianawi commented

2019-03-02 15:20:01 +00:00

(Migrated from github.com)

No description provided.

matthijskooijman (Migrated from github.com) requested changes

2019-03-11 19:30:51 +00:00

matthijskooijman (Migrated from github.com) left a comment

Code looks good, I only have some stylistic comments on the help text itself. If you fix those, I think this can be merged.

system/languages/en.php

I think each line of this helptext should start with a capital letter? And I think there is no point in having spaces before each \n?

I also don't really like hardcoding this list of possible page types, since we (or users) might add new datatypes which are then not reflected here. However, to properly solve this, datatypes must be able to provide their own helptext fragment somehow. But that is complicated, so until we have that, this is probably a fine approach (but we should create an issue to track this once this is merged).

I think each line of this helptext should start with a capital letter? And I think there is no point in having spaces before each `\n`? I also don't really like hardcoding this list of possible page types, since we (or users) might add new datatypes which are then not reflected here. However, to properly solve this, datatypes must be able to provide their own helptext fragment somehow. But that is complicated, so until we have that, this is probably a fine approach (but we should create an issue to track this once this is merged).

matthijskooijman commented

2019-03-14 19:32:10 +00:00

(Migrated from github.com)

I think this fixes #107, so it might be good to note that in the commit message (using the github auto-close syntax).

laurensmartina commented

2019-03-30 12:39:10 +00:00

(Migrated from github.com)

Fixed all, please review again.

matthijskooijman commented

2019-04-08 21:00:50 +00:00

(Migrated from github.com)

Looks good, I think this can be merged after testing.

matthijskooijman (Migrated from github.com) approved these changes

2019-04-08 21:02:50 +00:00

matthijskooijman commented

2019-04-09 08:40:12 +00:00

(Migrated from github.com)

I made one more textual change (the help text said 'artikel met peer reviewed', while the dropdown it explains says 'artikel met review') and rebased this on top of master. Merging now.

No reviewers

matthijskooijman

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

harmen/hypha!223

No description provided.

Rows
Columns