Skip to content
This repository was archived by the owner on Jan 25, 2021. It is now read-only.

Comments

Cassiopeia/tags x and font size#250

Merged
richard67 merged 8 commits intodevelopmentfrom
cassiopeia/tags-x-and-font-size
Nov 23, 2020
Merged

Cassiopeia/tags x and font size#250
richard67 merged 8 commits intodevelopmentfrom
cassiopeia/tags-x-and-font-size

Conversation

@hans2103
Copy link
Collaborator

Summary of Changes

This PR will convert the px values of choices css into relative sizes.
This PR will also increase font size selected tags.
This PR will also fix the position of the x in the multi select

Testing Instructions

  • Login frontend
  • Edit an article
  • Go to tab "Publishing"
  • Notice the position of the x at categories and notice the styling of present tags (add if none are present)
  • Apply patch
  • run npm run build:css
  • Refresh page you are editing.

Expected result

Schermafbeelding 2020-11-22 om 13 41 36

Actual result

Schermafbeelding 2020-11-22 om 13 42 26

Documentation Changes Required

@richard67
Copy link
Member

I've just tested.

Before applying PR, issue with Category "x" button see red mark:
2020-11-22_01

After applying PR:
2020-11-22_02

After applying PR with a tag with some letter in name which goes below the base line, i.e. g, j, q, y:
2020-11-22_03
=> Just acceptable, but a little bit more margin (or padding or whatever) between text and the frame would be better, I think.

@hans2103 @chmst @drmenzelit What do you think?

@hans2103
Copy link
Collaborator Author

hans2103 commented Nov 22, 2020

added inline-flex to improve alignment
Schermafbeelding 2020-11-22 om 16 26 41

@richard67
Copy link
Member

@hans2103 Still a bit too little padding at top and bottom of the text, for my taste.

@hans2103
Copy link
Collaborator Author

Schermafbeelding 2020-11-23 om 09 54 47
Schermafbeelding 2020-11-23 om 10 03 44

@hans2103 hans2103 requested review from chmst and richard67 November 23, 2020 09:04
@richard67
Copy link
Member

@hans2103 There is still some RTL missing. Note the difference between the 2 screenshots regarding the space between the "X" and the vertical line which separates it from the tag title:

  • LTR
    2020-11-23_01

  • RTL
    2020-11-23_02

@hans2103
Copy link
Collaborator Author

Padding fixed

Schermafbeelding 2020-11-23 om 19 27 05

Schermafbeelding 2020-11-23 om 19 27 19

@richard67
Copy link
Member

@hans2103 Now just a bit SCSS code style to be fixed. The linter finds following errors:

templates/cassiopeia/scss/vendor/choicesjs/choices.scss
 28:5  >  Expected "align-items" to come before "justify-content"   order/properties-order
 30:5  >  Expected "line-height" to come before "text-align"        order/properties-order

@richard67
Copy link
Member

@hans2103 I just see it's not only code style to be fixed. There is still a difference between LTR and RTL regarding the padding of the "x".

See following comparison, LTR at the top, RTL below that:

2020-11-23_04

@hans2103
Copy link
Collaborator Author

hans2103 commented Nov 23, 2020

joomla4 test_index php_typography_view=form layout=edit a_id=11 return=aHR0cDovL2pvb21sYTQudGVzdC9pbmRleC5waHAvdHlwb2dyYXBoeQ== (3)
joomla4 test_index php_typography_view=form layout=edit a_id=11 return=aHR0cDovL2pvb21sYTQudGVzdC9pbmRleC5waHAvdHlwb2dyYXBoeQ== (2)

@richard67
Copy link
Member

Looks equal now LTR and RTL:

2020-11-23_04

@richard67
Copy link
Member

The linter is happy now, too.

I have tested this PR ✅ with success.

@richard67 richard67 merged commit 8582807 into development Nov 23, 2020
@richard67 richard67 deleted the cassiopeia/tags-x-and-font-size branch November 23, 2020 19:06
@richard67
Copy link
Member

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants