Hi,
u:
> intrigeri:
>> Done. Everything looks fine to me except the actions toolbar change
>> that I've just reported in another sub-thread.
> I've fixed this now and made it much cleaner and nice IMO.
Indeed, I like it, it's look muuuuch more polished and precise to me!
One comment/question though: in RTL languages, the action toolbar
behaves in a somewhat weird way vs. breadcrumbs. It partly overlays
the breadcrumbs, which produces a weird result in itself, but I can
see some tiny bits of of breadcrumbs text in the empty/transparent
space between actions (e.g. between "Preferences" and "History"),
which is a bit confusing.
I've noticed this on doc/first_steps/persistence/configure.fa.html.
It's a regression compared to what we have on our live website at the
moment. I don't know if that's a blocker, but I want to make sure
you're taking this regression into account when requesting this
merge :) Your call!
This might also happen on LTR pages if the breadcrumbs are wide
enough, but I didn't check.
> So I hope it is mergeable now :)
Given the amount of comments sajolida made + a bunch of new changes
since he looked at the branch last, I'd like to see him check your
reply to his review + the current state of the branch, so I'm not
going to merge this myself.
Apart of the aforementioned RTL issue, I personally only have one tiny
comment left on this branch. Commit 7363d4a (that's really not atomic
enough for my taste BTW, but well, whatever) fixes some indentation
but introduces another buggy one:
@@ -887,7 +883,9 @@ background:#EEEEEE;
white-space: nowrap;
font-size: 0.9rem;
line-height: 2.2em;
+ height: 31px;
}
+
Thanks,
cheers!
--
intrigeri
