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.

Sign up for GitHub

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

Jump to bottom

feat: Modify bookinfo #1

Merged
merged 14 commits into from
Jul 19, 2022
Merged

feat: Modify bookinfo #1

merged 14 commits into from
Jul 19, 2022

Conversation

LXPWing
Copy link

@LXPWing LXPWing commented Jul 3, 2022 •
edited
Loading

  • redis state
  • oss img

@seeflood seeflood mentioned this pull request Jul 16, 2022
Closed
@LXPWing
Copy link
Author

LXPWing commented Jul 17, 2022

image
image

样式大概是这样，因为图片用的是相对定位不同的显示器可能会有差异。py调用java的preview service需要修改图2中的地址

@LXPWing
Copy link
Author

LXPWing commented Jul 17, 2022

istio的bookinfo页面，我打算重构下。（官方给的不够美观）

@seeflood
Copy link
Member

seeflood commented Jul 18, 2022 •
edited
Loading

棒！
但居然不是刻晴 😢

istio的bookinfo页面，我打算重构下。（官方给的不够美观）

恩但是有几点要注意的:

  • 我记得 bookinfo 会有一些官方测试，发http请求、判断返回结果中有没有某些字，从而判断是否请求成功、是否通过了 bookinfo 测试；所以要改页面的话，需要小心不会跑挂测试
  • UI 设计上，最好让 玩实验室的人能意识到“这是 istio 的那个 bookinfo、加了更多功能！”，让用户意识到这是 "istio++"；如果UI改的很花哨，我担心用户认不出来这是 "istio 的 bookinfo"了。这个比较主观，你自己拿捏吧~ 能让用户有 "istio++" 的感觉就行

@seeflood seeflood mentioned this pull request Jul 18, 2022
Closed
26 tasks
@LXPWing
Copy link
Author

LXPWing commented Jul 18, 2022

image
考虑了下，修改布局即可

@LXPWing LXPWing changed the title [WIP]feat: Modify bookinfo feat: Modify bookinfo Jul 18, 2022
@seeflood
Copy link
Member

@LXPWing Ok. 改布局要注意的是，页面右边还有评分栏（我理解你图上没有评分栏，是因为还没部署评分相关的系统？
image

@LXPWing
Copy link
Author

LXPWing commented Jul 18, 2022

评分栏是调用其他数据库的，本地测试的时候没有启动。

@seeflood
Copy link
Member

明白。我意思是 改布局是可以的，记得兼容右边栏就行

@seeflood
Copy link
Member

现在已经 ready 可以review了对吧?

@LXPWing
Copy link
Author

LXPWing commented Jul 18, 2022

评分栏在最下面独占一行，应该不会受到影响。可以review了

seeflood
seeflood requested changes Jul 19, 2022
View reviewed changes
samples/bookinfo/src/productpage/productpage.py Outdated Show resolved Hide resolved
samples/bookinfo/src/productpage/productpage.py Outdated Show resolved Hide resolved
seeflood
seeflood requested changes Jul 19, 2022
View reviewed changes
samples/bookinfo/src/productpage/templates/productpage.html Outdated
@@ -113,16 +122,28 @@ <h4 class="text-center text-primary">Book Details</h4>
<dt>Language:</dt>{{ details.language }}
<dt>ISBN-10:</dt>{{ details['ISBN-10'] }}
<dt>ISBN-13:</dt>{{ details['ISBN-13'] }}
{# show layotto state #}
<dt>news feed:</dt>{{ newsMsg }}
Copy link
Member

Choose a reason for hiding this comment

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

看起来 news feed 是作为“图书详情”表格里的一行？
news feed 做到图书详情的表格外面、作为单独的部分如何~ 如图：
image
因为 News Feed 有点像打开网页/app 后的一行广告，（类似于横幅广告），和当前的图书没关系，甚至就是和书无关的广告（比如“快来学做短视频，月入2万”）

@LXPWing
Copy link
Author

LXPWing commented Jul 19, 2022

image
news feed会不会很长

@seeflood
Copy link
Member

@LXPWing emmmmmm 那就限制下字数、缩小字体?

@LXPWing
Copy link
Author

LXPWing commented Jul 19, 2022 •
edited
Loading

我把news feed放到页面最下面然后做一个从右到左的滚动

效果和 https://blog.csdn.net/weixin_40845165/article/details/123629234 差不多

@seeflood
Copy link
Member

@LXPWing 啊这，所以是弹幕效果？
我ok. 要不先合并，等线上实验室搞出来看看效果，有问题到时候再优化

@LXPWing
Copy link
Author

LXPWing commented Jul 19, 2022

👌

@seeflood seeflood merged commit 3bb4492 into layotto:preview_service Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seeflood seeflood seeflood requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LXPWing @seeflood