Skip to content

feat: TagExtractor#1149

Merged
GeopJr merged 2 commits intomainfrom
feat/status/sep-tags
Oct 7, 2024
Merged

feat: TagExtractor#1149
GeopJr merged 2 commits intomainfrom
feat/status/sep-tags

Conversation

@GeopJr
Copy link
Owner

@GeopJr GeopJr commented Oct 7, 2024

fix: #880

I tried out different ways to do this and honestly this is probably the best.

  • Extracting while parsing the HTML

The way libxml2 traverses through a doc is unpredictable (aka, when it reaches an anchor, you can't exactly get the content, you have to go through it and usually the # character and the hashtag name are in different elements). That makes it impossible to detect that an anchor is a hashtag quickly and before parsing and appending it. Same thing for detecting if the last paragraph only has hashtags.

The structure also depends on the backend.

  • Extracting them afterwards by parsing the pango markup

While we now have a predictable structure, the same issue as above is still present. It becomes too difficult to reliably detect if the last paragraph is only made of hashtag anchors.

  • Current solution

Regex. (Anchors are predictable as it's being ran on the converted HTML)

Screencast.from.2024-10-07.06-43-17.mp4

@GeopJr GeopJr merged commit faf9aa7 into main Oct 7, 2024
@GeopJr GeopJr deleted the feat/status/sep-tags branch October 7, 2024 18:50
@bertob
Copy link
Contributor

bertob commented Oct 8, 2024

I haven't been able to build this myself yet due to a build error, but from looking at the video the tags look a bit too large/prominent. I'd try smaller font size and a bit less padding on them, and it'd be good to avoid the different widths.

I'm not quite sure what the options are there until we get AdwWrapLayout, but maybe we could do something like the Mastodon web UI and only show 4 by default, and the rest in a popover or something, so we don't have to deal with multiple rows for now?

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 8, 2024

it'd be good to avoid the different widths

(you are aware of flowbox' width situation, where column items have the same width which is the biggest item's. So the widths will be different when there are multiple rows)

but maybe we could do something like the Mastodon web UI and only show 4 by default, and the rest in a popover or something

The video is a bit quick but the implementation is the same as mastodon-web. It shows the first 3 and then I click the 'Show 5 More' button which causes them all to appear. No opinion on popover or even dialog

Here's some different styles:

Smaller font

Screenshot From 2024-10-08 11-37-16

Less padding + Smaller font

Screenshot From 2024-10-08 11-37-10

Normal font weight

Screenshot From 2024-10-08 11-36-32

Normal font weight + Less padding

Screenshot From 2024-10-08 11-36-23

Less padding

Screenshot From 2024-10-08 11-36-12

@bragefuglseth
Copy link
Contributor

(you are aware of flowbox' width situation, where column items have the same width which is the biggest item's. So the widths will be different when there are multiple rows)

FYI: this just landed in libadwaita’s main branch

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 9, 2024

Should I port it over? I have many use cases for WrapBox that are currently using FlowBox here

@bragefuglseth
Copy link
Contributor

Sure, it would be good to get some in-the-wild usage of the widget early on! And it seems like an obvious choice for this particular thing.

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 9, 2024

After spending some time porting it, I'm not sure if I want to continue.

We could use LabelWithWidgets in the meantime however:

Screencast.From.2024-10-09.09-33-29.mp4

since it's not really made just for custom emojis, but for any widget in general

edit: (obv, the video above doesn't feature the styling the others have)

@bragefuglseth
Copy link
Contributor

That works for now (with more space between the elements, haha)

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 9, 2024

Performance is terrible, I'll stick to flowbox for now 🤷 :(

@bertob
Copy link
Contributor

bertob commented Oct 9, 2024

Less padding + Smaller font

This one looks good to me!

After spending some time porting it, I'm not sure if I want to continue.

If you encounter any issues with WrapBox, now would be a great time to provide feedback :)

@GeopJr
Copy link
Owner Author

GeopJr commented Oct 9, 2024

If you encounter any issues with WrapBox, now would be a great time to provide feedback :)

Nothing to do with WrapBox itself! Porting it to Vala is quite the challenge due to some incompatibilities between C and Vala and ripping it out of libadwaita as is requires copying over a lot of other things, so it's probably best to wait for 1.7

@bertob
Copy link
Contributor

bertob commented Oct 9, 2024

Ah yeah, that makes sense 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Request]: Support less prominent hashtags

3 participants