NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
6 May 2021

Pull Request for Issue #33573 and #33100 (comment)

Summary of Changes

Removal of the bold white on the top level sidebar menu

Testing Instructions

npm run build:css

Actual result BEFORE applying this Pull Request

Screenshot 2021-05-06 at 17 39 30

Expected result AFTER applying this Pull Request

Screenshot 2021-05-06 at 17 38 30

Documentation Changes Required

avatar PhilETaylor PhilETaylor - open - 6 May 2021
avatar PhilETaylor PhilETaylor - change - 6 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 6 May 2021
Category Administration Templates (admin) NPM Change
avatar PhilETaylor PhilETaylor - change - 6 May 2021
The description was changed
avatar PhilETaylor PhilETaylor - edited - 6 May 2021
avatar brianteeman
brianteeman - comment - 6 May 2021

I deliberately did not create a pull request as I was looking to see if anyone else had the problem or if it was just my 54year old eyes.

As it is right now people just test it does what it says and then it gets merged without any thought

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

As it is right now people just test it does what it says and then it gets merged without any thought

Are you saying we dont have good maintainers? LMAO.

This clearly needs changing, I already mentioned it, and your issue already had 5 thumbs up within a few hours (5 being half the number f people that visit Joomla's GitHub a day on average) - just test it and lets get it merged.

avatar Quy Quy - test_item - 6 May 2021 - Tested successfully
avatar Quy
Quy - comment - 6 May 2021

I have tested this item successfully on 4ec0f2d

Confirmed the issue/fix.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33582.

avatar PhilETaylor PhilETaylor - change - 6 May 2021
The description was changed
avatar PhilETaylor PhilETaylor - edited - 6 May 2021
avatar srishty-07 srishty-07 - test_item - 6 May 2021 - Tested successfully
avatar srishty-07
srishty-07 - comment - 6 May 2021

I have tested this item successfully on 4ec0f2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33582.

avatar Kostelano Kostelano - test_item - 6 May 2021 - Tested successfully
avatar Kostelano
Kostelano - comment - 6 May 2021

I have tested this item successfully on 4ec0f2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33582.

avatar alikon alikon - change - 6 May 2021
Status Pending Ready to Commit
avatar alikon
alikon - comment - 6 May 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33582.

avatar brianteeman
brianteeman - comment - 6 May 2021

As expected this has been made RTC because of two tests and will now presumably be merged. Without anyone checking with the designer @ciar4n or any other experienced frontend designers to see if its the correct thing to do.

And yes there have been many pull requests recently that have been marked RTC only for a more experienced (or more observant) person come along to say that its wrong.

avatar Quy Quy - change - 6 May 2021
Labels Added: ?
avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Calm down it's not rocket science. It's a bold text. It can be easily reverted!

It's up to maintainers - no one else - to decide what gets merged. That's their role. All I've done is give them two choices.

avatar brianteeman
brianteeman - comment - 6 May 2021

That's why we have codeowners but no one looks at that

avatar alikon
alikon - comment - 6 May 2021

this pr has 3 tests
it's more than enough for me to set it as RTC

avatar chmst
chmst - comment - 6 May 2021

Thanks for the friendly and motivating words you have for us.

avatar HLeithner
HLeithner - comment - 6 May 2021

Guys what's your problem? RTC means nothing except that it is tested and both of you know this.

Stop this hate speech, thanks.

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Excuse me? Where is the hate speech you are accusing me of? I see no hate speech from me and you accusation is not acceptable.

Maybe you need to learn what hate speech is.

avatar PhilETaylor PhilETaylor - change - 6 May 2021
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2021-05-06 19:15:44
Closed_By PhilETaylor
Labels Added: ?
avatar PhilETaylor PhilETaylor - close - 6 May 2021
avatar HLeithner
HLeithner - comment - 6 May 2021

Maybe you should learn what you say?

Are you saying we dont have good maintainers? LMAO.

That's insulting.

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

No I was accusing Teeman of saying we dont have good maintainers...which is EXACTLY what he was implying and you have taken it out of context and called it "insulting" and "hate speech"

You are putting words in my mouth.

Maybe lost in translation. Teeman is just pissed because someone else did this PR instead of him. I dont care. Joomla can be shipped looking like crap if you want it to, its not like anyone actually uses Joomla anymore...

avatar HLeithner
HLeithner - comment - 6 May 2021

I can't do more then telling you and brian that it hurt's the people behind the term "maintainers".

The PR is good, because bold sucks but the conversation was not nice.

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

But Brian doesn't want this PR merged until others can talk about it to death. Its a bold text, its not rocket science. Joomla moves so slowly because decisions on stuff like this take forever - and yes, maintainers and release leads are VERY slow at making decisions on controversial PRs.

However, I might not aways agree with you closing/rejecting my PR's however, I will always defend your right AS A MAINTAINER to do so. Thats your role not mine. I know I'm the lowest of the low and no one gives a damn what I have to say, I have no role apart from the lowest in this project. Ive been here a long time, arguably (and Im not certain, I would need to get the old SVN Logs off the computer in storage) probably longer than teeman himself... and still my opinion counts for nothing - I know that.

Im sorry if you were offended by my comment, personally, that was not my intention at all.

avatar chmst
chmst - comment - 6 May 2021

Thank you.
Speaking for myself only: a "thank you" also for maintainers, not only for contributors is not a bad idea.

The PR is good and it was not rejected as PR. Could you re-open?

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Could you re-open?

Like I said. Im the lowest of the low. I do not even have permissions to reopen my own issues/prs. I hold no position here.

Screenshot 2021-05-06 at 21 22 21

avatar bembelimen
bembelimen - comment - 6 May 2021

grafik

That's the reason, why you can't reopen it.

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Nope - Ive never been able to reopen issues/pr's once closed.

I have repushed the branch, still cannot reopen the PR.

avatar alikon alikon - change - 6 May 2021
Status Closed New
Closed_Date 2021-05-06 19:15:44
Closed_By PhilETaylor
Labels Added: NPM Resource Changed
Removed: ?
avatar alikon alikon - change - 6 May 2021
Status New Pending
avatar alikon alikon - reopen - 6 May 2021
avatar alikon
alikon - comment - 6 May 2021

wow i've that power ?

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Like I said. Im the lowest of the low. I know my place.

avatar chmst
chmst - comment - 6 May 2021

lol ...thank you all

avatar brianteeman
brianteeman - comment - 6 May 2021

I say thank you to the maintainers every single time they merge a pr. Something that almost no one was doing at all until I started and now the majority do.

My concern is simply that PR are getting marked RTC and merged simply based on tests and not necessarily on need.

As I had raised this issue in the repaint PR but no one other than Phil commented I did not know if it was "just my eyes" or not so wanted to get some feedback from people who know better about design stuff before a pr was created. It was nothing to do with someone else creating the pr.

In the past purely design based issues such as the base font size have resulted in a lot of comments and I wanted to get the opinion before asking people to test. That is all. Nothing more. No conspiracy theory

avatar PhilETaylor
PhilETaylor - comment - 6 May 2021

Honestly a bold font in a colour I cannot control is the least of my issues at the moment...

Screenshot 2021-05-06 at 22 40 30

avatar ciar4n
ciar4n - comment - 7 May 2021

Honestly a bold font in a colour I cannot control is the least of my issues at the moment...

The template color changers make little or no since and have pretty random effects.

avatar ciar4n
ciar4n - comment - 7 May 2021

Well my personal prefrernece is bold as it offers some distinction between parent and child items in the menu. Also imo just fits better within the design.

Regardless, Im not presious about it. If my opinion on this is in the minority, I can live with the change :)

Before this pr

image

After this pr

image

avatar Fedik
Fedik - comment - 7 May 2021

That is old font rendering issue Mac vs Other ?
Mac usually looks bolder than other world.

The fix is okay :)

avatar richard67
richard67 - comment - 7 May 2021

So what to do now with this PR? Set RTC because it has 2 tests? Or organise a poll in the community what people like more? Or let release lead decide? Or throw a coin?

avatar PhilETaylor
PhilETaylor - comment - 7 May 2021

Firefox on mac renders with a different colour profile to most browsers (google it) and so colours look more vibrant.

Firefox on left, safari on right

Screenshot 2021-05-07 at 10 31 49

Hmm hard to see in a screenshot, but easy to see on a £5000 iMac Retina screen.

avatar alikon
alikon - comment - 7 May 2021

please folks
can we focus on "real" issues that prevent j4 to be released
instead of .... Ohh that "blue" is more "blue" than "blue" should be less "blue" ????

avatar PhilETaylor
PhilETaylor - comment - 7 May 2021

can we focus on "real" issues that prevent j4 to be released

Like what exactly?

avatar alikon
alikon - comment - 7 May 2021

this one is a perfect example

avatar PhilETaylor
PhilETaylor - comment - 7 May 2021

Pleased to say on the MacBook Pro laptop that I have been working on today remotely. The bold colour is a lot less in your face than it is on the iMac. Dramatically so. I guess there is a lot of variation based on the machine and the browser used.

avatar brianteeman
brianteeman - comment - 7 May 2021

from what I was reading its also down to if the text size is an integer or not

avatar bembelimen bembelimen - close - 7 May 2021
avatar bembelimen bembelimen - merge - 7 May 2021
avatar bembelimen bembelimen - change - 7 May 2021
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-07 20:15:52
Closed_By bembelimen
avatar bembelimen
bembelimen - comment - 7 May 2021

As this was already RTC and no code changes were made, it's good to merge.

Thanks for the PR

Add a Comment

Login with GitHub to post a comment