Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Responsive Meta Panel #525

Merged
merged 15 commits into from
Jul 29, 2024

Conversation

SilvanVerhoeven
Copy link
Collaborator

@SilvanVerhoeven SilvanVerhoeven commented Feb 22, 2024

Closes #275, closes #329, closes #53
Replaces stale #285

@SilvanVerhoeven SilvanVerhoeven marked this pull request as draft February 22, 2024 11:16
@SilvanVerhoeven SilvanVerhoeven changed the title Draft: Responsive Meta Panel Responsive Meta Panel Feb 22, 2024
@SilvanVerhoeven SilvanVerhoeven force-pushed the responsive-meta-panel branch 4 times, most recently from d3b336d to eb01573 Compare February 22, 2024 13:05
@SilvanVerhoeven SilvanVerhoeven marked this pull request as ready for review February 22, 2024 13:08
Copy link
Contributor

@frcroth frcroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Headings on minutes look like this for me:
image

myhpi/core/templates/core/minutes.html Outdated Show resolved Hide resolved
myhpi/core/templates/core/minutes.html Show resolved Hide resolved
myhpi/static/js/myHPI.js Show resolved Hide resolved
myhpi/static/js/navbar.js Show resolved Hide resolved
@SilvanVerhoeven
Copy link
Collaborator Author

Headings on minutes look like this for me: image

Correct, those are permalinks to the headings (so one can share a link that jumps to specific section on the page)

@jeriox
Copy link
Contributor

jeriox commented Feb 23, 2024

grafik
the content is not using the full width of the screen here

@jeriox
Copy link
Contributor

jeriox commented Feb 23, 2024

Correct, those are permalinks to the headings (so one can share a link that jumps to specific section on the page)

I like the idea of having those, but I'm also not so happy with the proposed layout. I don't think that most people get that the # after the title is meant to be the permalink. And the arrow-out-of-the-box-icon is only used for external links, but not internal ones. So I think its odd that is now part of another internal link.

@abc013
Copy link
Contributor

abc013 commented Mar 16, 2024

I like the idea of having those, but I'm also not so happy with the proposed layout. I don't think that most people get that the # after the title is meant to be the permalink. And the arrow-out-of-the-box-icon is only used for external links, but not internal ones. So I think its odd that is now part of another internal link.

(Just to give some additional Senf to the Wurst)

Personally, I really like GitHub's approach to that: Just showing a 'link' icon behind/before the title should be enough imho (It's hover-on on GitHub, but doesn't have to be here).
image

@SilvanVerhoeven
Copy link
Collaborator Author

the arrow-out-of-the-box-icon is only used for external links, but not internal ones. So I think its odd that is now part of another internal link.

@jeriox Agreed, but I am not sure I can do much about this at that point. Currently, all links not created by internal references get this arrow icon. I think this should be reworked in general in another branch

@SilvanVerhoeven
Copy link
Collaborator Author

Personally, I really like GitHub's approach to that: Just showing a 'link' icon behind/before the title should be enough imho (It's hover-on on GitHub, but doesn't have to be here).

Unfortunately, the used permanent link extension only allows to insert strings. The UTF 8 link emoji looks horrible on the latest windows 10, that's why we keep the # for now.

@dropforge
Copy link
Collaborator

Cannot reproduce the broken permalinks @frcroth

@SilvanVerhoeven SilvanVerhoeven force-pushed the responsive-meta-panel branch 3 times, most recently from 43d6d42 to be211c1 Compare July 29, 2024 18:55
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@SilvanVerhoeven SilvanVerhoeven requested review from JesterPandemonium and removed request for JesterPandemonium July 29, 2024 20:18
Copy link
Contributor

@abc013 abc013 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seem to be some issues, nonetheless :(
image
Also looks like the icons are cut off at the edges
image

@abc013
Copy link
Contributor

abc013 commented Jul 29, 2024

Poll pages still seem to have the old layout in the smartphone version

Copy link
Contributor

@abc013 abc013 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise looking good!

@SilvanVerhoeven SilvanVerhoeven merged commit 66fdf27 into fsr-de:main Jul 29, 2024
8 checks passed
@SilvanVerhoeven
Copy link
Collaborator Author

Abbreveations overlay button

Opened #587 for this

Look like the icons are cut off at the edges

This varies depending on screen size, resolution and browser. This is probably not an issue introduced by this PR

@SilvanVerhoeven
Copy link
Collaborator Author

SilvanVerhoeven commented Jul 29, 2024

Poll pages still seem to have the old layout in the smartphone version

Opened #588 for this

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

Successfully merging this pull request may close these issues.

Navigating using TOC does not account for navbar Proper layout for minutes on mobile Fixed sidebar
5 participants