-
Notifications
You must be signed in to change notification settings - Fork 221
Conversation
There was an idea from @demoive and @everton-rosario to change the taxonomy and remove the high level "Footer" section. I personally like the way @sakatam did it here for the simplicity and because it allows a nice future expansion to include other footer elements and options there (setting up Credits for instance). Please @everton-rosario and @demoive add your reasoning to this thread so it is documented. I'll approve the diff as it is and defer to @sakatam for the final decision on changing the taxonomy or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Please consider @demoive's and @everton-rosario's points on the taxonomy.
Right, so from the requests I've seen, the reason to have the option for a global footer setting is to make it easy for setting the content of the footer globally. In this sense, I classify it as a setting for the "theme" of an article (or "template") which lends itself for being under the "Styles" section. @everton-rosario made a point that if this were the case, perhaps "Styles" could be renamed to something else more encompassing, for example:
|
To @diegoquinteiro's point, however, in the future, I could also foresee the desire to define multiple footers, each to be applied to articles based on some condition — such as based on a category or tag. In this case, it could make sense to have a dedicated section for these settings, i.e. keep things as they are. |
Either way, I think this implementation is spot on. Great job here. |
Thank you guys all for your suggestions! I agree with moving the copyright setting into the "Style" section and renaming the section to more encompassing one. Once we will need to expand the footer setting in the future, we can split it and have a dedicated section. |
d6f42e0
to
cff9e07
Compare
fixes #261
- [x] Set footer element during the construction of IA document
- [x] Rename "Style" setting to "Appearance"
- [x] Consolidate appearance settings logic into one function
cff9e07
to
3d3a4b4
Compare
Pushed the new version. Now the copyright setting is in the new "Appearance" section which includes Style and RTL settings. |
@diegoquinteiro @demoive @everton-rosario Do you guys have any remaining concerns on this change? |
Looks great. Merging. =) |
Just updated to the latest version of the plugin and don't see this option?? |
@samichamoun Unfortunately, this one is not in v3.3. Will definitely be included in the next release! |
Ah, no worries. I notice that the screenshot says no tags can be added. Is there a reason why we can't put any hyperlinks in here as I've seen a number of pages using the footer for this? |
I've been inserting footers manually in every article which is tedious, plus they get replaced every time the article updates. Is there a way to do this at the PHP level with hyperlink tags? Completely new to programming. |
This is simply due to the restriction of the SDK that this plugin depends on. However, since our platform allows hyperlinks in here, we should be able to do that within this plugin, too. I will file an issue for this. Thank you for pointing this out! |
Filed an issue for rich formats: #619 |
fixes #261