#93 Ony print feed_title in combined mode if it is actually defined.

Closed
Maru wants to merge 1 commits from Maru/master into fox/master
Maru commented 11 months ago

Hi fox,

Initially I wanted to handle this topic via discourse but the system did not allow me to create an account. This change actually only print the feed_title if it is defined which gets around blank space if you start working with CSS styles. Initially I tried a CSS only approach but found this to be a lot easier.

Kind regards, Michael

Hi fox, Initially I wanted to handle this topic via discourse but the system did not allow me to create an account. This change actually only print the feed_title if it is defined which gets around blank space if you start working with CSS styles. Initially I tried a CSS only approach but found this to be a lot easier. Kind regards, Michael
fox commented 11 months ago
Owner

this looks way less readable tbh. why would you need this?

also i'm completely not enjoying you ignoring my code style

e: also, why single out this particular field?

this looks way less readable tbh. why would you need this? also i'm completely not enjoying you ignoring my code style e: also, why single out this particular field?
fox commented 11 months ago
Owner

alright,

1) i'm not adding special case markup for what i assume is a theme. that's just not happening.

2) there's a JS-side pluginhost hook which is passed the resulting rendered article DOM tree, if you need to modify markup it would be very easy to go this way and do whatever you want with it, in a generic way.

if you feel that plugin hooks are insufficient (or broken, they might be after recent changes) i'd be happy to help. this way this works for everyone else, not just you specifically.

otherwise next week someone will come asking to hide a different field and a few days later my clean markup is going to be a bloated nightmare.

alright, 1) i'm not adding special case markup for what i assume is a theme. that's just not happening. 2) there's a JS-side pluginhost hook which is passed the resulting rendered article DOM tree, if you need to modify markup it would be very easy to go this way and do whatever you want with it, in a generic way. if you feel that plugin hooks are insufficient (or broken, they might be after recent changes) i'd be happy to help. this way this works for everyone else, not just you specifically. otherwise next week someone will come asking to hide a different field and a few days later my clean markup is going to be a bloated nightmare.
Maru commented 11 months ago
Poster

I want to use the new flex layout of the header to re-oder the fields. One of the changes is to move the feed name from the back to the front. I manage this with the following code. ..... .cdm div.header div.feed a { display: inline-block; margin-top: 1px; border-radius: 99px; color: #444; line-height: 1; max-width: 115px; overflow: hidden; text-overflow: ellipsis; }

div#floatingTitle .feed, .cdm div.header div.feed { flex: 0 0 135px; order: 2; }

I more or less make it a fixed length of 135px and cut the text before. This gives a nice block layout. This worked fine prior to the last changes because the feed_title was apparently not always printed. With the latests changes it is fixed and so I get a blank line instead.

I want to use the new flex layout of the header to re-oder the fields. One of the changes is to move the feed name from the back to the front. I manage this with the following code. ..... .cdm div.header div.feed a { display: inline-block; margin-top: 1px; border-radius: 99px; color: #444; line-height: 1; max-width: 115px; overflow: hidden; text-overflow: ellipsis; } div#floatingTitle .feed, .cdm div.header div.feed { flex: 0 0 135px; order: 2; } I more or less make it a fixed length of 135px and cut the text before. This gives a nice block layout. This worked fine prior to the last changes because the feed_title was apparently not always printed. With the latests changes it is fixed and so I get a blank line instead.
Maru commented 11 months ago
Poster

Just saw your last comment while writing my initial reply. I will take a look at the hook. Of course it would be even better if I could make this change strictly with CSS but for now I did not succeed.

Just saw your last comment while writing my initial reply. I will take a look at the hook. Of course it would be even better if I could make this change strictly with CSS but for now I did not succeed.
fox commented 11 months ago
Owner

i don't think you can match :text or whatever length with css (yet) but a plugin hook would be very easy to write.

also: why wouldn't discourse let you register?

i don't think you can match :text or whatever length with css (yet) but a plugin hook would be very easy to write. also: why wouldn't discourse let you register?
Maru commented 11 months ago
Poster

Of course registering works now, before that I got an error along the lines of max access from same IP. I now tried it again to get the error message for you and of course it works now. Sorry for the noise.

Of course registering works now, before that I got an error along the lines of max access from same IP. I now tried it again to get the error message for you and of course it works now. Sorry for the noise.
fox commented 11 months ago
Owner

oh that's probably because of cloudflare. i'll take a look at discourse settings, thanks for reporting this.

oh that's probably because of cloudflare. i'll take a look at discourse settings, thanks for reporting this.
Please reopen this pull request to perform merge operation.
Sign in to join this conversation.
No Label
No Milestone
No assignee
2 Participants
Loading...
Cancel
Save
There is no content yet.