Add link underline for main body; add link icon #175

Closed

bitkeks wants to merge 2 commits from feature/underlined_links into master

bitkeks commented

2018-08-18 22:58:02 +00:00

(Migrated from github.com)

Links in the main body part are now underlined on mouse hover.
tags in the article content are expanded by an icon from
FontAwesome.

Fixes #140

Links in the main body part are now underlined on mouse hover. <a> tags in the article content are expanded by an icon from FontAwesome. Fixes #140

elegaanz (Migrated from github.com) requested changes 2018-08-31 15:29:13 +00:00

elegaanz (Migrated from github.com) left a comment

I have two little remarks:

I don't know if an icon next to each link is necessary, is there any real reason to add them?
There is an indentation issue in the CSS, it should be padded by only two spaces.

Otherwise, it looks good. 👍

I have two little remarks: - I don't know if an icon next to each link is necessary, is there any real reason to add them? - There is an indentation issue in the CSS, it should be padded by only two spaces. Otherwise, it looks good. :+1:

Madeorsk commented

2018-09-01 13:09:07 +00:00

(Migrated from github.com)

Adding an icon next to a link can be done in CSS only, so I don't see the point of injecting an icon in the markdown parser, or do I misunderstand something?

👍 1

bitkeks commented

2018-09-03 08:24:44 +00:00

(Migrated from github.com)

I don't know if an icon next to each link is necessary, is there any real reason to add them?

No other than enhanced visibility!

There is an indentation issue in the CSS, it should be padded by only two spaces.

My editor shows me tab characters, with a width of 8 spaces?

Adding an icon next to a link can be done in CSS only, so I don't see the point of injecting an icon in the markdown parser

Yeah, might be the best solution if any icons should be used. The injection was a fix to use the FontAwesome icon, I didn't find a CSS-only solution.

> I don't know if an icon next to each link is necessary, is there any real reason to add them? No other than enhanced visibility! > There is an indentation issue in the CSS, it should be padded by only two spaces. My editor shows me tab characters, with a width of 8 spaces? > Adding an icon next to a link can be done in CSS only, so I don't see the point of injecting an icon in the markdown parser Yeah, might be the best solution if any icons should be used. The injection was a fix to use the FontAwesome icon, I didn't find a CSS-only solution.

elegaanz (Migrated from github.com) approved these changes 2018-09-03 09:02:23 +00:00

elegaanz (Migrated from github.com) left a comment

Looks good! (but there are conflicts now 😑)

For the CSS only solution, I think something with ::after and content should be possible.

Looks good! (but there are conflicts now :expressionless:) For the CSS only solution, I think something with `::after` and `content` should be possible.

Madeorsk commented

2018-09-03 14:50:24 +00:00

(Migrated from github.com)

Yes, if you look on the FontAwesome website, it indicates the character used for a specific icon. Then you just have to apply the FontAwesome font to this (look into the FontAwesome css file if you want, there is an example of how you can do this! :-))

Should I make a PR with an icon next to the links?

👍 1

elegaanz commented

2018-09-03 15:11:44 +00:00

(Migrated from github.com)

@Madeorsk I don't think it is really useful, it is more bloating the interface and making the text harder to read than anything else IMHO.

elegaanz commented

2018-09-29 17:37:37 +00:00

(Migrated from github.com)

@bitkeks Hello! Do you plan to fix the conflicts? It's OK if you don't want, but tell us, so that we can close this PR and make it clear nobody is working on this issue anymore. 🙂

@bitkeks Hello! Do you plan to fix the conflicts? It's OK if you don't want, but tell us, so that we can close this PR and make it clear nobody is working on this issue anymore. :slightly_smiling_face:

trinity-1686a commented

2018-09-29 19:45:04 +00:00

Owner

If needed, I have what I think is a successful rebase to master here

If needed, I have what I think is a successful rebase to master [here](https://github.com/fdb-hiroshima/Plume/tree/feature/underlined_links)

👍 1 ❤️ 1

bitkeks commented

2018-09-30 12:55:20 +00:00

(Migrated from github.com)

Sorry people, this issue must have been gone under the radar..
Feel free to close this one and merge from @fdb-hiroshima, I'm not sure if I have enough time to cleanly resolve the conflicts to current master. 🙂

Sorry people, this issue must have been gone under the radar.. Feel free to close this one and merge from @fdb-hiroshima, I'm not sure if I have enough time to cleanly resolve the conflicts to current master. :slightly_smiling_face:

👍 1

elegaanz commented

2018-09-30 12:57:46 +00:00

(Migrated from github.com)

Ok, thanks for keeping us informed. 🙂

Ok, thanks for keeping us informed. :slightly_smiling_face:

Pull request closed

This pull request cannot be reopened because the branch was deleted.

No reviewers

elegaanz

No milestone

No project

No assignees

2 participants

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: Plume/Plume#175

No description provided.

Rows
Columns