pleroma/pleroma-fe
Watch
2
2
Fork
You've already forked pleroma-fe
 10
Code Issues 351 Pull requests 26 Projects Releases 5 Packages 1 Wiki Activity

Fix the bug with 3-dot menu #2080

Closed
tae wants to merge 2 commits from gitlab-mr-iid-811 into develop
pull from: gitlab-mr-iid-811
merge into: pleroma:develop
Conversation 16 Commits 2 Files changed 3 +60 -61
tae commented 2019-05-26 17:37:05 +00:00
Member
Copy link

closes #559

closes #559
hj commented 2019-05-26 17:58:43 +00:00
Member
Copy link

can it be kept in popper.css and imported here?

can it be kept in popper.css and imported here?
tae commented 2019-05-26 18:18:39 +00:00
Author
Member
Copy link

isn't it weird to include the CSS file in a nested folder here?

isn't it weird to include the CSS file in a nested folder here?
hj commented 2019-05-26 18:24:16 +00:00
Member
Copy link

maybe make entire popper automatically imported then?

maybe make entire popper automatically imported then?
tae commented 2019-05-26 18:29:07 +00:00
Author
Member
Copy link

hmm, I don't understand.

hmm, I don't understand.
hj commented 2019-05-26 19:15:41 +00:00
Member
Copy link

The bug most likely happens (i didn't reproduced it yet, and never encountered it) because with webpack 4 components are loaded on-demand, or at least seems that way. A better example would be user-settings and, uh, just settings. They mostly share same styles, but styles themselves are declared in settings, so (before i fixed the bug) if you opened user-settings the page would be somewhat broken, you'd need to open up settings to get the styles and going back to user-settings would give you fixed page.

Seeing you move styles around makes me think this is the same case, so probably popper somehow either not imported correctly somewhere or styles are shared.

The bug most likely happens (i didn't reproduced it yet, and never encountered it) because with webpack 4 components are loaded on-demand, or at least seems that way. A better example would be `user-settings` and, uh, just `settings`. They mostly share same styles, but styles themselves are declared in `settings`, so (before i fixed the bug) if you opened `user-settings` the page would be somewhat broken, you'd need to open up `settings` to get the styles and going back to `user-settings` would give you fixed page. Seeing you move styles around makes me think this is the same case, so probably popper somehow either not imported correctly somewhere or styles are shared.
hj commented 2019-05-26 19:16:41 +00:00
Member
Copy link

if i understand correctly this should be moved to popper.css and it would fix it.

if i understand correctly this should be moved to popper.css and it would fix it.
tae commented 2019-05-26 20:28:00 +00:00
Author
Member
Copy link

I've just tried, but no luck. As long as ModerationTools is not mounted (ModerationTools is for admin only), nothing will be changed.

I've just tried, but no luck. As long as `ModerationTools` is not mounted (`ModerationTools` is for admin only), nothing will be changed.
hj commented 2019-05-26 21:14:46 +00:00
Member
Copy link

see #2083

see #2083
tae commented 2019-05-27 01:07:12 +00:00
Author
Member
Copy link

Well, thanks for opening a new MR with your solution. I've just checked it. But, I'm 100% sure that the popper.css will be injected to the browser twice (if you are admin).

The correct path to write shared css is to use global css file. Or to inject css when necessary, it's better to make a reusable component containing that css (if it is worth doing that).

Well, thanks for opening a new MR with your solution. I've just checked it. But, I'm 100% sure that the popper.css will be injected to the browser twice (if you are admin). The correct path to write shared css is to use global css file. Or to inject css when necessary, it's better to make a reusable component containing that css (if it is worth doing that).
hj commented 2019-05-27 05:28:35 +00:00
Member
Copy link

then just import it from app.scss as i mentioned previously.

then just import it from app.scss as i mentioned previously.
hj commented 2019-05-27 05:28:48 +00:00
Member
Copy link

also maybe check if it's actually injected twice

also maybe check if it's actually injected twice
tae commented 2019-05-27 12:14:52 +00:00
Author
Member
Copy link

I've just tested locally and confirmed that it is injected twice.

I've just tested locally and confirmed that it is injected twice.
tae commented 2019-05-27 12:17:57 +00:00
Author
Member
Copy link

then just import it from app.scss as i mentioned previously.

I really don't understand what benefit we have by doing that. I would like to know your thoughts @shpuld @lambadalambda

> then just import it from app.scss as i mentioned previously. I really don't understand what benefit we have by doing that. I would like to know your thoughts @shpuld @lambadalambda
hj commented 2019-05-27 14:05:08 +00:00
Member
Copy link

having less clutter in App.scss

having less clutter in App.scss
tae commented 2019-05-28 16:25:08 +00:00
Author
Member
Copy link

Ok, I've updated.

Ok, I've updated.
tae commented 2019-05-29 17:10:45 +00:00
Author
Member
Copy link

closed by #2083

closed by https://git.pleroma.social/pleroma/pleroma-fe/pulls/2083

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
BE Request

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1
Instance Admin Dashboard

  
NL2
Improved Mobile Experience

  
NL4
Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility
https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident
Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
pleroma/pleroma-fe!2080
No description provided.